Skip to content

Commit

Permalink
Merge commit 'fb323df64afbc798bf487d2af5a6e88a54de7dac'
Browse files Browse the repository at this point in the history
  • Loading branch information
Mirroring committed Jul 16, 2024
2 parents c188d43 + fb323df commit 509fea1
Show file tree
Hide file tree
Showing 115 changed files with 926 additions and 12,735 deletions.
2 changes: 2 additions & 0 deletions NuGet.config
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,9 @@
<clear />
<!--Begin: Package sources managed by Dependency Flow automation. Do not edit the sources below.-->
<!-- Begin: Package sources from dotnet-emsdk -->
<add key="darc-pub-dotnet-emsdk-be13dab" value="https://pkgs.dev.azure.com/dnceng/public/_packaging/darc-pub-dotnet-emsdk-be13dabf/nuget/v3/index.json" />
<add key="darc-pub-dotnet-emsdk-e92f92e" value="https://pkgs.dev.azure.com/dnceng/public/_packaging/darc-pub-dotnet-emsdk-e92f92ef/nuget/v3/index.json" />
<add key="darc-pub-dotnet-emsdk-abd5b69" value="https://pkgs.dev.azure.com/dnceng/public/_packaging/darc-pub-dotnet-emsdk-abd5b69d/nuget/v3/index.json" />
<!-- End: Package sources from dotnet-emsdk -->
<!-- Begin: Package sources from dotnet-sdk -->
<!-- End: Package sources from dotnet-sdk -->
Expand Down
14 changes: 3 additions & 11 deletions docs/design/features/dotnet-pgo.md
Original file line number Diff line number Diff line change
@@ -1,21 +1,13 @@
# dotnet-pgo Spec
Utilize trace data for improving application performance

NOTE: This documentation page contains information on some features that are still work-in-progress.

## Intro

The dotnet-pgo tool is a cross-platform CLI global tool that enables conversion of traces of .NET Core applications collected via dotnet-trace, ETW, perfview, perfcollect, LTTNG to be used to improve the performance of an application or library.

## Installing dotnet-pgo

The first step is to install the dotnet-pgo CLI global tool.

```cmd
$ dotnet tool install --global dotnet-pgo
You can invoke the tool using the following command: dotnet-pgo
Tool 'dotnet-pgo' (version '6.0.47001') was successfully installed.
```
The only way to use dotnet-pgo is to build it in the runtime repo. To learn how to build the runtime, consult the [how to build](https://github.com/dotnet/runtime/tree/main/docs/workflow/building/coreclr) docs for Windows, macOS, or Linux.

## Using dotnet-pgo to optimize an application

Expand All @@ -37,14 +29,14 @@ set DOTNET_TC_QuickJitForLoops=1
set DOTNET_TC_CallCountThreshold=10000
set DOTNET_ReadyToRun=0
dotnet-trace collect --providers Microsoft-Windows-DotNETRuntime:0x1E000080018:4 -- bin\Release\net6.0\pgotest.exe
dotnet-trace collect --providers Microsoft-Windows-DotNETRuntime:0x1E000080018:4 -- bin\Release\net{version-number-goes-here}.0\pgotest.exe
set DOTNET_TieredPGO=
set DOTNET_TC_QuickJitForLoops=
set DOTNET_TC_CallCountThreshold=
set DOTNET_ReadyToRun=
dotnet-pgo create-mibc --trace trace.nettrace --output trace.mibc
${YOUR-REPO-ROOT}\artifacts\bin\coreclr\{OS}.{ARCHITECTURE}.{CONFIGURATION}\dotnet-pgo create-mibc --trace trace.nettrace --output trace.mibc
dotnet publish --runtime win-x64 -p:PublishReadyToRun=true -p:ReadyToRunOptimizationData=trace.mibc
```
Expand Down
4 changes: 2 additions & 2 deletions eng/Version.Details.xml
Original file line number Diff line number Diff line change
Expand Up @@ -354,9 +354,9 @@
<Uri>https://dev.azure.com/dnceng/internal/_git/dotnet-optimization</Uri>
<Sha>67613417f5e1af250e6ddfba79f8f2885d8e90fb</Sha>
</Dependency>
<Dependency Name="Microsoft.DotNet.HotReload.Utils.Generator.BuildTool" Version="8.0.0-alpha.0.24271.1">
<Dependency Name="Microsoft.DotNet.HotReload.Utils.Generator.BuildTool" Version="8.0.0-alpha.0.24365.1">
<Uri>https://github.com/dotnet/hotreload-utils</Uri>
<Sha>c804541158619aae93105f54698ca7f149d28232</Sha>
<Sha>9fd9805b21b21e16d1c81f0ffabd6ee81fe3e8a2</Sha>
</Dependency>
<Dependency Name="System.Runtime.Numerics.TestData" Version="8.0.0-beta.24270.1">
<Uri>https://github.com/dotnet/runtime-assets</Uri>
Expand Down
2 changes: 1 addition & 1 deletion eng/Versions.props
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,7 @@
<MicrosoftDotNetXHarnessTestRunnersCommonVersion>8.0.0-prerelease.24229.2</MicrosoftDotNetXHarnessTestRunnersCommonVersion>
<MicrosoftDotNetXHarnessTestRunnersXunitVersion>8.0.0-prerelease.24229.2</MicrosoftDotNetXHarnessTestRunnersXunitVersion>
<MicrosoftDotNetXHarnessCLIVersion>8.0.0-prerelease.24229.2</MicrosoftDotNetXHarnessCLIVersion>
<MicrosoftDotNetHotReloadUtilsGeneratorBuildToolVersion>8.0.0-alpha.0.24271.1</MicrosoftDotNetHotReloadUtilsGeneratorBuildToolVersion>
<MicrosoftDotNetHotReloadUtilsGeneratorBuildToolVersion>8.0.0-alpha.0.24365.1</MicrosoftDotNetHotReloadUtilsGeneratorBuildToolVersion>
<XUnitVersion>2.4.2</XUnitVersion>
<XUnitAnalyzersVersion>1.0.0</XUnitAnalyzersVersion>
<XUnitRunnerVisualStudioVersion>2.4.5</XUnitRunnerVisualStudioVersion>
Expand Down
4 changes: 2 additions & 2 deletions eng/docker/libraries-sdk.linux.Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ FROM $SDK_BASE_IMAGE as target

ARG VERSION=8.0
ARG CONFIGURATION=Release
ENV _DOTNET_INSTALL_CHANNEL="$VERSION.1xx"
ENV _DOTNET_INSTALL_CHANNEL=$VERSION

# Install latest daily SDK:
RUN wget https://dot.net/v1/dotnet-install.sh
Expand Down Expand Up @@ -52,4 +52,4 @@ COPY --from=corefxbuild \
ENV _ASPNETCORE_SOURCE="/usr/share/dotnet/shared/Microsoft.AspNetCore.App/$VERSION*"
ENV _ASPNETCORE_DEST="/live-runtime-artifacts/testhost/net$VERSION-linux-$CONFIGURATION-x64/shared/Microsoft.AspNetCore.App"
RUN mkdir -p $_ASPNETCORE_DEST
RUN cp -r $_ASPNETCORE_SOURCE $_ASPNETCORE_DEST
RUN cp -r $_ASPNETCORE_SOURCE $_ASPNETCORE_DEST
4 changes: 2 additions & 2 deletions eng/docker/libraries-sdk.windows.Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ FROM $SDK_BASE_IMAGE as target
SHELL ["pwsh", "-Command", "$ErrorActionPreference = 'Stop'; $ProgressPreference = 'SilentlyContinue';"]

ARG VERSION=8.0
ENV _DOTNET_INSTALL_CHANNEL="$VERSION.1xx"
ENV _DOTNET_INSTALL_CHANNEL=$VERSION
ARG CONFIGURATION=Release

USER ContainerAdministrator
Expand All @@ -22,4 +22,4 @@ COPY . /live-runtime-artifacts
ENV _ASPNETCORE_SOURCE="C:/Program Files/dotnet/shared/Microsoft.AspNetCore.App/$VERSION*"
ENV _ASPNETCORE_DEST="C:/live-runtime-artifacts/testhost/net$VERSION-windows-$CONFIGURATION-x64/shared/Microsoft.AspNetCore.App"
RUN & New-Item -ItemType Directory -Path $env:_ASPNETCORE_DEST
RUN Copy-Item -Recurse -Path $env:_ASPNETCORE_SOURCE -Destination $env:_ASPNETCORE_DEST
RUN Copy-Item -Recurse -Path $env:_ASPNETCORE_SOURCE -Destination $env:_ASPNETCORE_DEST
9 changes: 7 additions & 2 deletions eng/pipelines/common/macos-sign-with-entitlements.yml
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,15 @@ steps:
includeRootFolder: true
replaceExistingArchive: true

- task: SFP.build-tasks.custom-build-task-1.EsrpCodeSigning@1
- task: EsrpCodeSigning@5
displayName: 'ESRP CodeSigning'
inputs:
ConnectedServiceName: 'ESRP CodeSigning'
ConnectedServiceName: 'DotNet-Engineering-Services_KeyVault'
AppRegistrationClientId: '28ec6507-2167-4eaa-a294-34408cf5dd0e'
AppRegistrationTenantId: '72f988bf-86f1-41af-91ab-2d7cd011db47'
AuthAKVName: 'EngKeyVault'
AuthCertName: 'DotNetCore-ESRP-AuthCert'
AuthSignCertName: 'DotNetCore-ESRP-AuthSignCert'
FolderPath: '$(Build.ArtifactStagingDirectory)/'
Pattern: 'mac_entitled_to_sign.zip'
UseMinimatch: true
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,12 @@
<?xml version="1.0" encoding="utf-8"?>
<!-- https://learn.microsoft.com/en-us/dotnet/fundamentals/package-validation/diagnostic-ids -->
<Suppressions xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xmlns:xsd="http://www.w3.org/2001/XMLSchema">
<Suppression>
<DiagnosticId>CP0002</DiagnosticId>
<Target>M:System.Reflection.Assembly.SetEntryAssembly(System.Reflection.Assembly)</Target>
<Left>ref/net8.0/System.Private.CoreLib.dll</Left>
<Right>lib/net8.0/System.Private.CoreLib.dll</Right>
</Suppression>
<Suppression>
<DiagnosticId>CP0015</DiagnosticId>
<Target>M:System.Diagnostics.Tracing.EventSource.Write``1(System.String,``0):[T:System.Diagnostics.CodeAnalysis.RequiresUnreferencedCodeAttribute]</Target>
Expand Down
6 changes: 6 additions & 0 deletions src/coreclr/jit/block.h
Original file line number Diff line number Diff line change
Expand Up @@ -1883,6 +1883,12 @@ class AllSuccessorEnumerator
return m_block;
}

// True if NextSuccessor will return the first successor or null
bool AtStart() const
{
return m_curSucc == UINT_MAX;
}

// Returns the next available successor or `nullptr` if there are no more successors.
BasicBlock* NextSuccessor(Compiler* comp)
{
Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/jit/compiler.h
Original file line number Diff line number Diff line change
Expand Up @@ -6334,7 +6334,7 @@ class Compiler
PhaseStatus optCloneLoops();
void optCloneLoop(unsigned loopInd, LoopCloneContext* context);
PhaseStatus optUnrollLoops(); // Unrolls loops (needs to have cost info)
void optRemoveRedundantZeroInits();
void optRemoveRedundantZeroInits(bool hasCycle);
PhaseStatus optIfConversion(); // If conversion

protected:
Expand Down
50 changes: 38 additions & 12 deletions src/coreclr/jit/optimizer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -9007,6 +9007,9 @@ typedef JitHashTable<unsigned, JitSmallPrimitiveKeyFuncs<unsigned>, unsigned> Lc
//------------------------------------------------------------------------------------------
// optRemoveRedundantZeroInits: Remove redundant zero initializations.
//
// Arguments:
// hasCycle -- true if SSA's topological sort detected a cycle in the flowgraph
//
// Notes:
// This phase iterates over basic blocks starting with the first basic block until there is no unique
// basic block successor or until it detects a loop. It keeps track of local nodes it encounters.
Expand All @@ -9024,8 +9027,10 @@ typedef JitHashTable<unsigned, JitSmallPrimitiveKeyFuncs<unsigned>, unsigned> Lc
// either the local has no gc pointers or there are no gc-safe points between the prolog and the assignment,
// then the local is marked with lvHasExplicitInit which tells the codegen not to insert zero initialization
// for this local in the prolog.

void Compiler::optRemoveRedundantZeroInits()
//
// It depends on pre/post order numbers set by TopologicalSort.
//
void Compiler::optRemoveRedundantZeroInits(bool hasCycle)
{
#ifdef DEBUG
if (verbose)
Expand All @@ -9043,10 +9048,37 @@ void Compiler::optRemoveRedundantZeroInits()

assert(fgNodeThreading == NodeThreading::AllTrees);

for (BasicBlock* block = fgFirstBB; (block != nullptr) && ((block->bbFlags & BBF_MARKED) == 0);
block = block->GetUniqueSucc())
for (BasicBlock* block = fgFirstBB; (block != nullptr); block = block->GetUniqueSucc())
{
block->bbFlags |= BBF_MARKED;
if (hasCycle)
{
// See if this block is a cycle entry
//
bool stop = false;
for (FlowEdge* predEdge = BlockPredsWithEH(block); predEdge != nullptr;
predEdge = predEdge->getNextPredEdge())
{
BasicBlock* const predBlock = predEdge->getSourceBlock();

if ((block->bbPreorderNum <= predBlock->bbPreorderNum) &&
(predBlock->bbPostorderNum <= block->bbPostorderNum))
{
JITDUMP(FMT_BB " is part of a cycle, stopping the block scan\n", block->bbNum);
stop = true;
break;
}
}

// If so, stop looking for redundant zero inits
//
if (stop)
{
break;
}
}

JITDUMP("Analyzing " FMT_BB "\n", block->bbNum);

CompAllocator allocator(getAllocator(CMK_ZeroInit));
LclVarRefCounts defsInBlock(allocator);
bool removedTrackedDefs = false;
Expand Down Expand Up @@ -9167,7 +9199,7 @@ void Compiler::optRemoveRedundantZeroInits()

if (tree->Data()->IsIntegralConst(0))
{
bool bbInALoop = (block->bbFlags & BBF_BACKWARD_JUMP) != 0;
bool bbInALoop = false;
bool bbIsReturn = block->bbJumpKind == BBJ_RETURN;

if (!bbInALoop || bbIsReturn)
Expand Down Expand Up @@ -9244,12 +9276,6 @@ void Compiler::optRemoveRedundantZeroInits()
}
}
}

for (BasicBlock* block = fgFirstBB; (block != nullptr) && ((block->bbFlags & BBF_MARKED) != 0);
block = block->GetUniqueSucc())
{
block->bbFlags &= ~BBF_MARKED;
}
}

//------------------------------------------------------------------------
Expand Down
23 changes: 20 additions & 3 deletions src/coreclr/jit/ssabuilder.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,7 @@ SsaBuilder::SsaBuilder(Compiler* pCompiler)
, m_allocator(pCompiler->getAllocator(CMK_SSA))
, m_visitedTraits(0, pCompiler) // at this point we do not know the size, SetupBBRoot can add a block
, m_renameStack(m_allocator, pCompiler->lvaCount)
, m_hasCycle(false)
{
}

Expand Down Expand Up @@ -179,6 +180,7 @@ int SsaBuilder::TopologicalSort(BasicBlock** postOrder, int count)
};

// Compute order.
int preIndex = 0;
int postIndex = 0;
BasicBlock* block = comp->fgFirstBB;
BitVecOps::AddElemD(&m_visitedTraits, m_visited, block->bbNum);
Expand All @@ -189,11 +191,26 @@ int SsaBuilder::TopologicalSort(BasicBlock** postOrder, int count)

while (!blocks.Empty())
{
BasicBlock* block = blocks.TopRef().Block();
BasicBlock* succ = blocks.TopRef().NextSuccessor(comp);
AllSuccessorEnumerator& e = blocks.TopRef();
BasicBlock* block = e.Block();

if (e.AtStart())
{
DBG_SSA_JITDUMP("[SsaBuilder::TopologicalSort] preOrder[%d] = " FMT_BB "\n", preIndex, block->bbNum);
block->bbPreorderNum = preIndex;
block->bbPostorderNum = UINT_MAX;
preIndex++;
}

BasicBlock* succ = e.NextSuccessor(comp);

if (succ != nullptr)
{
if ((succ->bbPreorderNum <= block->bbPreorderNum) && (succ->bbPostorderNum == UINT_MAX))
{
m_hasCycle = true;
}

// if the block on TOS still has unreached successors, visit them
if (BitVecOps::TryAddElemD(&m_visitedTraits, m_visited, succ->bbNum))
{
Expand Down Expand Up @@ -1541,7 +1558,7 @@ void SsaBuilder::Build()
m_pCompiler->fgLocalVarLiveness();
EndPhase(PHASE_BUILD_SSA_LIVENESS);

m_pCompiler->optRemoveRedundantZeroInits();
m_pCompiler->optRemoveRedundantZeroInits(m_hasCycle);
EndPhase(PHASE_ZERO_INITS);

// Mark all variables that will be tracked by SSA
Expand Down
2 changes: 2 additions & 0 deletions src/coreclr/jit/ssabuilder.h
Original file line number Diff line number Diff line change
Expand Up @@ -101,4 +101,6 @@ class SsaBuilder
BitVec m_visited;

SsaRenameState m_renameStack;

bool m_hasCycle;
};
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
<?xml version="1.0" encoding="utf-8"?>
<!-- https://learn.microsoft.com/en-us/dotnet/fundamentals/package-validation/diagnostic-ids -->
<Suppressions xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xmlns:xsd="http://www.w3.org/2001/XMLSchema">
<Suppression>
<DiagnosticId>CP0001</DiagnosticId>
Expand Down Expand Up @@ -960,6 +961,12 @@
<DiagnosticId>CP0002</DiagnosticId>
<Target>M:System.TypedReference.get_IsNull</Target>
</Suppression>
<Suppression>
<DiagnosticId>CP0002</DiagnosticId>
<Target>M:System.Reflection.Assembly.SetEntryAssembly(System.Reflection.Assembly)</Target>
<Left>ref/net8.0/System.Private.CoreLib.dll</Left>
<Right>lib/net8.0/System.Private.CoreLib.dll</Right>
</Suppression>
<Suppression>
<DiagnosticId>CP0014</DiagnosticId>
<Target>M:System.Runtime.CompilerServices.RuntimeHelpers.GetObjectValue(System.Object)-&gt;object?:[T:System.Diagnostics.CodeAnalysis.NotNullIfNotNullAttribute]</Target>
Expand Down
21 changes: 15 additions & 6 deletions src/coreclr/pal/src/exception/machexception.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -681,8 +681,8 @@ HijackFaultingThread(
BuildExceptionRecord(exceptionInfo, &exceptionRecord);

#if defined(HOST_AMD64)
threadContext.ContextFlags = CONTEXT_FLOATING_POINT;
CONTEXT_GetThreadContextFromThreadState(x86_FLOAT_STATE, (thread_state_t)&exceptionInfo.FloatState, &threadContext);
threadContext.ContextFlags = CONTEXT_FLOATING_POINT | CONTEXT_XSTATE;
CONTEXT_GetThreadContextFromThreadState(x86_AVX512_STATE, (thread_state_t)&exceptionInfo.FloatState, &threadContext);

threadContext.ContextFlags |= CONTEXT_CONTROL | CONTEXT_INTEGER | CONTEXT_SEGMENTS;
CONTEXT_GetThreadContextFromThreadState(x86_THREAD_STATE, (thread_state_t)&exceptionInfo.ThreadState, &threadContext);
Expand Down Expand Up @@ -1265,10 +1265,19 @@ MachExceptionInfo::MachExceptionInfo(mach_port_t thread, MachMessage& message)
machret = thread_get_state(thread, x86_THREAD_STATE, (thread_state_t)&ThreadState, &count);
CHECK_MACH("thread_get_state", machret);

count = x86_FLOAT_STATE_COUNT;
machret = thread_get_state(thread, x86_FLOAT_STATE, (thread_state_t)&FloatState, &count);
CHECK_MACH("thread_get_state(float)", machret);

count = x86_AVX512_STATE_COUNT;
machret = thread_get_state(thread, x86_AVX512_STATE, (thread_state_t)&FloatState, &count);
if (machret != KERN_SUCCESS)
{
count = x86_AVX_STATE_COUNT;
machret = thread_get_state(thread, x86_AVX_STATE, (thread_state_t)&FloatState, &count);
if (machret != KERN_SUCCESS)
{
count = x86_FLOAT_STATE_COUNT;
machret = thread_get_state(thread, x86_FLOAT_STATE, (thread_state_t)&FloatState, &count);
CHECK_MACH("thread_get_state(float)", machret);
}
}
count = x86_DEBUG_STATE_COUNT;
machret = thread_get_state(thread, x86_DEBUG_STATE, (thread_state_t)&DebugState, &count);
CHECK_MACH("thread_get_state(debug)", machret);
Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/pal/src/exception/machmessage.h
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ struct MachExceptionInfo
mach_exception_data_type_t Subcodes[2];
#if defined(HOST_AMD64)
x86_thread_state_t ThreadState;
x86_float_state_t FloatState;
x86_avx512_state_t FloatState;
x86_debug_state_t DebugState;
#elif defined(HOST_ARM64)
arm_thread_state64_t ThreadState;
Expand Down
12 changes: 12 additions & 0 deletions src/coreclr/pal/src/thread/context.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1513,6 +1513,18 @@ CONTEXT_GetThreadContextFromThreadState(
CONTEXT_GetThreadContextFromThreadState((thread_state_flavor_t)pState->fsh.flavor, (thread_state_t)&pState->ufs, lpContext);
}
break;
case x86_AVX_STATE:
{
x86_avx_state_t *pState = (x86_avx_state_t *)threadState;
CONTEXT_GetThreadContextFromThreadState((thread_state_flavor_t)pState->ash.flavor, (thread_state_t)&pState->ufs, lpContext);
}
break;
case x86_AVX512_STATE:
{
x86_avx512_state_t *pState = (x86_avx512_state_t *)threadState;
CONTEXT_GetThreadContextFromThreadState((thread_state_flavor_t)pState->ash.flavor, (thread_state_t)&pState->ufs, lpContext);
}
break;
#elif defined(HOST_ARM64)
case ARM_THREAD_STATE64:
if (lpContext->ContextFlags & (CONTEXT_CONTROL | CONTEXT_INTEGER) & CONTEXT_AREA_MASK)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
<TargetFrameworks>$(NetCoreAppCurrent);$(NetCoreAppPrevious);$(NetCoreAppMinimum);netstandard2.0;$(NetFrameworkMinimum)</TargetFrameworks>
<EnableDefaultItems>true</EnableDefaultItems>
<IsPackable>true</IsPackable>
<GeneratePackageOnBuild>true</GeneratePackageOnBuild>
<GeneratePackageOnBuild>false</GeneratePackageOnBuild>
<ServicingVersion>2</ServicingVersion>
<PackageDescription>Provides the functionality to bind an object to data in configuration providers for Microsoft.Extensions.Configuration. This package enables you to represent the configuration data as strongly-typed classes defined in the application code. To bind a configuration, use the Microsoft.Extensions.Configuration.ConfigurationBinder.Get extension method on the IConfiguration object. To use this package, you also need to install a package for the configuration provider, for example, Microsoft.Extensions.Configuration.Json for the JSON provider.</PackageDescription>
</PropertyGroup>
Expand Down
Loading

0 comments on commit 509fea1

Please sign in to comment.