[llvm-dev] Diff to add ARMv6L to Target parser

[Artyom Skrobov via llvm-dev]

LGTM; thanks!

From: William Dillon [mailto:william at housedillon.com]
Sent: 11 January 2016 20:02
To: Artyom Skrobov
Cc: Eric Christopher; Renato Golin; Tim Northover; LLVM Dev; nd; Daniel Sanders; Frederic Riss
Subject: Re: [llvm-dev] Diff to add ARMv6L to Target parser

Good catch, Thanks.

Here’s the updated patch:

Index: lib/Support/TargetParser.cpp
===================================================================
--- lib/Support/TargetParser.cpp               (revision 257090)
+++ lib/Support/TargetParser.cpp             (working copy)
@@ -400,7 +400,7 @@
   return StringSwitch<StringRef>(Arch)
       .Case("v5", "v5t")
       .Case("v5e", "v5te")
-      .Case("v6j", "v6")
+      .Case("v6j", "v6l", "v6")
       .Case("v6hl", "v6k")
       .Cases("v6m", "v6sm", "v6s-m", "v6-m")
       .Cases("v6z", "v6zk", "v6kz")
Index: unittests/ADT/TripleTest.cpp
===================================================================
--- unittests/ADT/TripleTest.cpp              (revision 257090)
+++ unittests/ADT/TripleTest.cpp            (working copy)
@@ -851,6 +851,10 @@
     EXPECT_EQ("arm1136jf-s", Triple.getARMCPUForArch());
   }
   {
+    llvm::Triple Triple("armv6l-unknown-eabi");
+    EXPECT_EQ("arm1136jf-s", Triple.getARMCPUForArch());
+  }
+  {
     llvm::Triple Triple("armv6j-unknown-eabi");
     EXPECT_EQ("arm1136jf-s", Triple.getARMCPUForArch());
   }


On Jan 11, 2016, at 1:37 AM, Artyom Skrobov <Artyom.Skrobov at arm.com<mailto:Artyom.Skrobov at arm.com>> wrote:

Hello William – I have a minor comment on your patch:

       .Case("v6j", "v6")
      .Case("v6l", "v6")

may be written more concisely as

       .Cases("v6j", "v6l", "v6")


From: William Dillon [mailto:william at housedillon.com]
Sent: 08 January 2016 18:56
To: Bob Wilson
Cc: Eric Christopher; Artyom Skrobov; Renato Golin; Tim Northover; LLVM Dev; nd; Daniel Sanders; Frederic Riss
Subject: Re: [llvm-dev] Diff to add ARMv6L to Target parser

Thanks for the clarifications, Bob!

I’ve spent some time with the head of the llvm.org<http://llvm.org/> repo, and I now understand a lot better what Renato and Tim were talking about re. the architecture aliases.  The patch to add v6l, therefore, seems simple enough.  I haven’t been able to test it in my usual flow, because that involves the whole swift stack.  I’m considering creating a program that links to llvm to test the behavior.

Index: lib/Support/TargetParser.cpp
===================================================================
--- lib/Support/TargetParser.cpp               (revision 257090)
+++ lib/Support/TargetParser.cpp             (working copy)
@@ -401,6 +401,7 @@
       .Case("v5", "v5t")
       .Case("v5e", "v5te")
       .Case("v6j", "v6")
+      .Case("v6l", "v6")
       .Case("v6hl", "v6k")
       .Cases("v6m", "v6sm", "v6s-m", "v6-m")
       .Cases("v6z", "v6zk", "v6kz")
Index: unittests/ADT/TripleTest.cpp
===================================================================
--- unittests/ADT/TripleTest.cpp              (revision 257090)
+++ unittests/ADT/TripleTest.cpp            (working copy)
@@ -851,6 +851,10 @@
     EXPECT_EQ("arm1136jf-s", Triple.getARMCPUForArch());
   }
   {
+    llvm::Triple Triple("armv6l-unknown-eabi");
+    EXPECT_EQ("arm1136jf-s", Triple.getARMCPUForArch());
+  }
+  {
     llvm::Triple Triple("armv6j-unknown-eabi");
     EXPECT_EQ("arm1136jf-s", Triple.getARMCPUForArch());
   }


I looked into the tests (and unit tests), and found that perhaps the most appropriate test to add is in the determination of the cpu type from the architecture version.  I verified that ARM1136 is the base for ARM11, which is all that can be assumed given armv6.  That jives with the existing armv6 architecture test.  I added another copy of that for armv6l.  I assume that this will sufficiently test whether armv6l is aliasing correctly to armv6.

Anyway, thanks again.
- Will
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20160112/22237605/attachment-0001.html>