[arm32 unix building] Passing target platform and os to IlcCompiler for cross compiling #2793

sergign60 · 2017-02-22T09:22:48Z

This pull request contains some fixes for test cross building on arm32 systems.

There are several problems:

Some linux on arm does not give information about processor cache sizes. There are no the directories /sys/devices/system/cpu/cpu*/cache and lscpu does not show this info also. I've checked out for example
Linux raspberrypi 4.1.19-v7+ Remove Encoding workaround #858 SMP armv7l GNU/Linux and
Linux odroid 4.9.6 Add .gitattributes and .gitignore files #3 SMP PREEMPT armv7l armv7l armv7l GNU/Linux
There are no implementation for src/ILCompiler.Compiler/src/Compiler/DependencyAnalysis/Target_ARM/ARMReadyToRunHelperNode.cs: EmitCode. After I comment throw Exception in this method the issue [arm32 unix] The test Hello fails on arm32 because of difference between initializing and using structures #2780 disappears so it can be closed if this pull request is accepted
There is another issue with the Hello test on arm32 about string processing. It is under investigation.

dnfclas · 2017-02-22T09:22:52Z

Hi @sergign60, I'm your friendly neighborhood .NET Foundation Pull Request Bot (You can call me DNFBOT). Thanks for your contribution!
You've already signed the contribution license agreement. Thanks!

The agreement was validated by .NET Foundation and real humans are currently evaluating your PR.

TTYL, DNFBOT;

sergign60 · 2017-02-22T09:23:12Z

CC: @jkotas @Dmitri-Botcharnikov

jkotas · 2017-02-22T09:32:00Z

src/ILCompiler.Compiler/src/Compiler/DependencyAnalysis/Target_ARM/TargetRegisterMap.cs

            switch (os)
            {
                case TargetOS.Windows:
+                case TargetOS.Linux:


ARM calling convention is same on all OSes. It would be better to delete this switch.

@jkotas changed

jkotas · 2017-02-22T09:34:17Z

tests/src/Simple/SimpleTest.targets

  </ItemGroup>

+  <ItemGroup>
+    <IlsArg Include="--targetarch=$(Platform)" />


@jkotas
of course it's misprinting. I've tested 'IlcArg'

jkotas · 2017-02-22T09:37:04Z

src/Native/Runtime/unix/PalRedhawkUnix.cpp

+    if (g_cbLargestOnDieCache == 0)
    {
-        if ((strncmp(cpuEntry->d_name, "cpu", 3) == 0) && isdigit(cpuEntry->d_name[3]))
+        DIR* cpuDir = opendir("/sys/devices/system/cpu");


@janvorli I do not see a similar logic in CoreCLR. Is there a good reason for having it in CoreRT?

This additional logic theoretically enables getting cache size on non-intel processors (intel ones should be handled by the sysconf stuff).
But I am not sure if the /sys/devices/system/cpu contains cache details on other architectures at all. On my Raspberry Pi 3, both with aarch64 and arm32 Linux distros, the cache details are not in there and I remember that @sdmaclea said that on his aarch64 Linux device, there were no cache details either.
@sergign60 are these present on your arm32 Linux distro / device?

@janvorli I've written about it above - two arm-based systems
Linux raspberrypi 4.1.19-v7+ #858 SMP armv7l GNU/Linux and
Linux odroid 4.9.6 #3 SMP PREEMPT armv7l armv7l armv7l GNU/Linux
that I've checked out doesn't have this information. I guess that one of the possible and simplest way is to include into corert the table with info about arm processors and get out it from this table.

@janvorli Should we delete this parsing of sys/devices/system/cpu then?

I would recommend deleting it. We can introduce it in the future if we support targets that provide this information.

@sergign60 Could you please do it as part of this change?

@jkotas @janvorli Should I close parsing sys/devices/system/cpu for arm processors only?

jkotas · 2017-02-22T09:37:48Z

src/ILCompiler/src/Program.cs

+                    _targetArchitecture = TargetArchitecture.ARMEL;
+                else if (_targetArchitectureStr.Equals("arm64"))
+                    _targetArchitecture = TargetArchitecture.ARM64;
+                else


This should be command line exception with message

@jkotas changed

jkotas · 2017-02-22T09:42:11Z

src/ILCompiler/src/Program.cs

+            // Set target Architecture and OS
+            //
+            if (_targetArchitectureStr != null)
+            {


It maybe nice to do the comparisons case-insensitive for convenience. _targetArchitectureStr.Equals("x86", StringComparison.OrdinalIgnoreCase)

@jkotas added

jkotas · 2017-02-22T09:45:07Z

src/ILCompiler.Compiler/src/Compiler/DependencyAnalysis/Target_ARM/ARMReadyToRunHelperNode.cs:EmitCode.

Are you hitting this method with CppCodeGen? This method should not be called with CppCodeGen at all.

sergign60 · 2017-02-22T12:13:15Z

@jkotas
the mistake mentioned by me arises when

       IlcCompile:
         Creating directory "/home/signatov/CORERT/corert/tests/src/Simple/Hello/obj/Debug/armel/native/".
         "/home/signatov/CORERT/corert/tests/../bin/Product/Linux.armel.Debug/packaging/publish1/corerun" "/home/signatov/CORERT/corert/tests/../bin/Product/Linux.armel.Debug/packaging/publish1/ilc.dll" @"/home/signatov/CORERT/corert/tests/src/Simple/Hello/obj/Debug/armel/native/Hello.ilc.rsp"
         
         Unhandled Exception: System.NotImplementedException: The method or operation is not implemented.
            at ILCompiler.DependencyAnalysis.ReadyToRunHelperNode.EmitCode(NodeFactory factory, ARMEmitter& encoder, Boolean relocsOnly)
            at ILCompiler.DependencyAnalysis.AssemblyStubNode.GetData(NodeFactory factory, Boolean relocsOnly)
            at ILCompiler.DependencyAnalysis.ObjectNode.GetStaticDependencies(NodeFactory factory)
            at ILCompiler.DependencyAnalysisFramework.DependencyAnalyzer`2.GetStaticDependenciesImpl(DependencyNodeCore`1 node)
            at ILCompiler.DependencyAnalysisFramework.DependencyAnalyzer`2.GetStaticDependencies(DependencyNodeCore`1 node)
            at ILCompiler.DependencyAnalysisFramework.DependencyAnalyzer`2.ProcessMarkStack()
            at ILCompiler.DependencyAnalysisFramework.DependencyAnalyzer`2.ComputeMarkedNodes()
            at ILCompiler.DependencyAnalysisFramework.DependencyAnalyzer`2.get_MarkedNodeList()
            at ILCompiler.CppCodegenCompilation.CompileInternal(String outputFile)
            at ILCompiler.Compilation.ILCompiler.ICompilation.Compile(String outputFile)
            at ILCompiler.Program.Run(String[] args)
            at ILCompiler.Program.Main(String[] args)
         Aborted (core dumped)```

jkotas · 2017-02-22T16:07:48Z

@MichalStrehovsky We end up creating AssembyStubNodes during CppCodeGen. Where would be the best place to cut it off?

MichalStrehovsky · 2017-02-22T17:21:26Z

@MichalStrehovsky We end up creating AssembyStubNodes during CppCodeGen. Where would be the best place to cut it off?

This is two things (both of the brought in from EETypeNode):

Unboxing stubs referenced from vtables of valuetypes. AFAIK we don't have an implementation of the unboxing stubs for CppCodegen, so we end up with the machine code stub that is probably thrown away. This is done in CppCodegenFactory.cs around the // TODO: this is wrong line.
Virtual call R2R helpers. EETypeNode currently drags these in based on seeing a VirtualMethodUse for these. They end up being unused for CppCodegen. We probably need a better factoring.

I could try to come up with a way to avoid this (it would be a general improvement), but it might be easier to just not throw from the stubs and have them generate machine code that crashes at runtime.

jkotas · 2017-02-27T16:50:44Z

LGTM. Thank!

dnfclas added the cla-already-signed label Feb 22, 2017

sergign60 mentioned this pull request Feb 22, 2017

[arm32 unix] The test Hello fails on arm32 because of difference between initializing and using structures #2780

Closed

jkotas reviewed Feb 22, 2017

View reviewed changes

sergign60 added 3 commits February 27, 2017 14:11

Passing target platform&os to IlcCompiler

ec0d7d1

Some fixes added

010ee8e

Provided runtime assert instead of compile-time

4dacc6d

sergign60 force-pushed the arm_build branch from 64f0130 to 4dacc6d Compare February 27, 2017 11:13

sergign60 added 3 commits February 27, 2017 15:01

Deleted /sys/devices/system/cpu/cache parsing

eaafff6

Additional to deleting /sys/devices/system/cpu/cache parsing

57a3c9c

Added empty version of InteropThunksHelpers for arm

87f8c2e

jkotas merged commit 3e057ae into dotnet:master Feb 27, 2017

sergign60 deleted the arm_build branch February 27, 2017 17:32

[arm32 unix building] Passing target platform and os to IlcCompiler for cross compiling #2793

[arm32 unix building] Passing target platform and os to IlcCompiler for cross compiling #2793

Uh oh!

Conversation

sergign60 commented Feb 22, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dnfclas commented Feb 22, 2017

Uh oh!

sergign60 commented Feb 22, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sergign60 Feb 22, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sergign60 Feb 24, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jkotas commented Feb 22, 2017

Uh oh!

sergign60 commented Feb 22, 2017

Uh oh!

jkotas commented Feb 22, 2017

Uh oh!

MichalStrehovsky commented Feb 22, 2017

Uh oh!

jkotas commented Feb 27, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

sergign60 commented Feb 22, 2017 •

edited

Loading

sergign60 Feb 22, 2017 •

edited

Loading

sergign60 Feb 24, 2017 •

edited

Loading