Skip to content

Commit

Permalink
Improve crossgen2 comparison jobs (#44119)
Browse files Browse the repository at this point in the history
- Fix compilation on unix platforms
  - Wrap use of wildcard in quotes
- Print better display name into log
- Fix X86 constant comparison handling
- Add ability to compile specific overload via single method switches
  • Loading branch information
davidwrighton authored Nov 9, 2020
1 parent d4c1c9a commit 6548832
Show file tree
Hide file tree
Showing 7 changed files with 75 additions and 6 deletions.
4 changes: 4 additions & 0 deletions eng/pipelines/coreclr/templates/crossgen2-comparison-job.yml
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,8 @@ jobs:
value: ${{ parameters.targetarch }}
- name: crossgencompare_build_artifact
value: crossgen_comparison_build_${{ parameters.targetos }}_${{ parameters.targetarch }}
- name: displayname_comparison_job
value: ${{ format('Test crossgen2-comparison {0}{1} {2} {3} to {4} {5}', parameters.osGroup, parameters.osSubgroup, parameters.archType, parameters.buildConfig, parameters.targetarch, parameters.targetos) }}
- ${{ if eq(parameters.targetos, 'windows') }}:
- name: target_crossgen2_os
value: windows
Expand Down Expand Up @@ -138,6 +140,7 @@ jobs:
CorrelationPayloadDirectory: '$(Build.SourcesDirectory)/src/tests/Common/scripts'
${{ if ne(parameters.osGroup, 'windows') }}:
WorkItemCommand:
echo $(displayname_comparison_job) ;
echo Targeting $(targetFlavor) ;
chmod +x $HELIX_WORKITEM_PAYLOAD/corerun;
mkdir -p $HELIX_WORKITEM_PAYLOAD/log;
Expand All @@ -154,6 +157,7 @@ jobs:
--diff_dir $HELIX_WORKITEM_PAYLOAD/log
${{ if eq(parameters.osGroup, 'windows') }}:
WorkItemCommand:
echo $(displayname_comparison_job) &
echo Targeting $(targetFlavor) &
md %HELIX_WORKITEM_PAYLOAD%\log &
set CORE_ROOT=%HELIX_CORRELATION_PAYLOAD% &
Expand Down
4 changes: 2 additions & 2 deletions src/coreclr/src/gcdump/i386/gcdumpx86.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -255,7 +255,7 @@ size_t GCDump::DumpGCTable(PTR_CBYTE table,

table = DumpEncoding(table, sz);

_ASSERTE(0 == ~OFFSET_MASK % sizeof(void*));
_ASSERTE(0 == ~OFFSET_MASK % sizeof(uint32_t));

lowBits = OFFSET_MASK & stkOffs;
stkOffs &= ~OFFSET_MASK;
Expand Down Expand Up @@ -304,7 +304,7 @@ size_t GCDump::DumpGCTable(PTR_CBYTE table,

DumpEncoding(bp, table-bp);

_ASSERTE(0 == ~OFFSET_MASK % sizeof(void*));
_ASSERTE(0 == ~OFFSET_MASK % sizeof(uint32_t));

lowBits = varOffs & 0x3;
varOffs &= ~OFFSET_MASK;
Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/src/jit/lower.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2549,7 +2549,7 @@ GenTree* Lowering::OptimizeConstCompare(GenTree* cmp)
// into ((x AND mask) NE|EQ 0) when mask is a single bit.
//

if (isPow2(static_cast<size_t>(op2Value)) && andOp2->IsIntegralConst(op2Value))
if (isPow2<target_size_t>(static_cast<target_size_t>(op2Value)) && andOp2->IsIntegralConst(op2Value))
{
op2Value = 0;
op2->SetIconValue(0);
Expand Down
2 changes: 2 additions & 0 deletions src/coreclr/src/tools/aot/crossgen2/CommandLineOptions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ internal class CommandLineOptions

public string SingleMethodTypeName;
public string SingleMethodName;
public int SingleMethodIndex;
public IReadOnlyList<string> SingleMethodGenericArg;

public IReadOnlyList<string> CodegenOptions;
Expand Down Expand Up @@ -107,6 +108,7 @@ public CommandLineOptions(string[] args)

syntax.DefineOption("singlemethodtypename", ref SingleMethodTypeName, SR.SingleMethodTypeName);
syntax.DefineOption("singlemethodname", ref SingleMethodName, SR.SingleMethodMethodName);
syntax.DefineOption("singlemethodindex", ref SingleMethodIndex, SR.SingleMethodIndex);
syntax.DefineOptionList("singlemethodgenericarg", ref SingleMethodGenericArg, SR.SingleMethodGenericArgs);

syntax.DefineOption("parallelism", ref Parallelism, SR.ParalellismOption);
Expand Down
59 changes: 58 additions & 1 deletion src/coreclr/src/tools/aot/crossgen2/Program.cs
Original file line number Diff line number Diff line change
Expand Up @@ -634,7 +634,50 @@ private MethodDesc CheckAndParseSingleMethodModeArguments(CompilerTypeSystemCont
TypeDesc owningType = FindType(context, _commandLineOptions.SingleMethodTypeName);

// TODO: allow specifying signature to distinguish overloads
MethodDesc method = owningType.GetMethod(_commandLineOptions.SingleMethodName, null);
MethodDesc method = null;
bool printMethodList = false;
int curIndex = 0;
foreach (var searchMethod in owningType.GetMethods())
{
if (searchMethod.Name != _commandLineOptions.SingleMethodName)
continue;

curIndex++;
if (_commandLineOptions.SingleMethodIndex != 0)
{
if (curIndex == _commandLineOptions.SingleMethodIndex)
{
method = searchMethod;
break;
}
}
else
{
if (method == null)
{
method = searchMethod;
}
else
{
printMethodList = true;
}
}
}

if (printMethodList)
{
curIndex = 0;
foreach (var searchMethod in owningType.GetMethods())
{
if (searchMethod.Name != _commandLineOptions.SingleMethodName)
continue;

curIndex++;
Console.WriteLine($"{curIndex} - {searchMethod}");
}
throw new CommandLineException(SR.SingleMethodIndexNeeded);
}

if (method == null)
throw new CommandLineException(string.Format(SR.MethodNotFoundOnType, _commandLineOptions.SingleMethodName, _commandLineOptions.SingleMethodTypeName));

Expand Down Expand Up @@ -666,6 +709,20 @@ private static bool DumpReproArguments(CodeGenerationFailedException ex)

Console.Write($"--singlemethodtypename \"{formatter.FormatName(failingMethod.OwningType, true)}\"");
Console.Write($" --singlemethodname {failingMethod.Name}");
{
int curIndex = 0;
foreach (var searchMethod in failingMethod.OwningType.GetMethods())
{
if (searchMethod.Name != failingMethod.Name)
continue;

curIndex++;
if (searchMethod == failingMethod.GetMethodDefinition())
{
Console.Write($" --singlemethodindex {curIndex}");
}
}
}

for (int i = 0; i < failingMethod.Instantiation.Length; i++)
Console.Write($" --singlemethodgenericarg \"{formatter.FormatName(failingMethod.Instantiation[i], true)}\"");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,10 @@
<value>Single method compilation: generic arguments to the method</value>
</data>
<data name="SingleMethodMethodName" xml:space="preserve">
<value>Single method compilation: generic arguments to the method</value>
<value>Single method compilation: method name</value>
</data>
<data name="SingleMethodIndex" xml:space="preserve">
<value>Single method compilation: Index of method if there are multiple with the same name</value>
</data>
<data name="SingleMethodTypeName" xml:space="preserve">
<value>Single method compilation: name of the owning type</value>
Expand All @@ -252,6 +255,9 @@
<data name="TypeAndMethodNameNeeded" xml:space="preserve">
<value>Both method name and type name are required parameters for single method mode</value>
</data>
<data name="SingleMethodIndexNeeded" xml:space="preserve">
<value>There are multiple methods with the same name. Specify --singlemethodindex [index] with the correct index.</value>
</data>
<data name="TypeNotFound" xml:space="preserve">
<value>Type '{0}' not found</value>
</data>
Expand Down
2 changes: 1 addition & 1 deletion src/tests/Common/scripts/crossgen2_comparison.py
Original file line number Diff line number Diff line change
Expand Up @@ -526,7 +526,7 @@ def _build_args_crossgen_il_file(self, il_filename, ni_filename, platform_assemb
args.append(self.dotnet)
args.append(self.crossgen_executable_filename)
args.append('-r')
args.append(platform_assemblies_paths + self.platform_directory_sep + '*.dll')
args.append('"' + platform_assemblies_paths + self.platform_directory_sep + '*.dll"' )
args.append('-O')
args.append('--out')
args.append(ni_filename)
Expand Down

0 comments on commit 6548832

Please sign in to comment.