On 08/28/2013 09:33 PM, Jeroen Hofstee wrote:
Hello Renato and others,

On 08/27/2013 11:20 AM, Renato Golin wrote:
On 27 August 2013 09:24, Dmitri Gribenko <[email protected] <mailto:[email protected]>> wrote:

    On Tue, Aug 27, 2013 at 1:11 AM, Renato Golin
    <[email protected] <mailto:[email protected]>> wrote:
    > I don't have that strong an opinion to keep things as they are,
    but I also
    > wouldn't change unless there is a good reason (or consensus) to
    do so.

    In my opinion, consistency is a pretty good reason.


That's where the grey area is... Do we strive to be consistent with the previous behaviour, or with other flags?

The former, how it's implemented now, is that a "strict-align" flag would taint the whole argument list, since there was no way to "un-taint". The latter, would change how "strict-align" behaves, being more consistent with other flags, but less consistent with its former self.

Jeroen, do you have any input in this? Any special reason for not having made like other dual flags?


No special reason, I must have been in a good mood I guess.
I would like it even more to throw an error at the user for passing
such inconsistent flags, with the question to make up his / her mind
and filter-out one of them. Or preferably not add them both in the
first place.

Luckily we don't have to discuss that, for the simple fact gcc seems
to use the last flag and clang seems to follow accordingly. Patches
attached, intended to match that behaviour.

follow up: Updated the subject and removed personal comment from the test.

Regards,
Jeroen

commit 773f3b50160944d735a8e91ea7071b928bbfe7f1
Author: Jeroen Hofstee <[email protected]>
Date:   Wed Aug 28 20:30:48 2013 +0200

    use the last passed -munaligned-access / -mno-unaligned-access
    
    Passing inconsistent munaligned-access / mno-unaligned-access
    flags, intentionally resulted in a warning and the flag
    no-unaligned-access being used.
    
    Gcc does, at least in practice, use the last flag in such a
    case. This patch updates clang behaviour accordingly; use the
    last flag or base alignment behaviour on the target (which
    llvm will do if no flag is explicitly passed)

diff --git lib/Driver/Tools.cpp lib/Driver/Tools.cpp
index 6862e76..9633d6b 100644
--- lib/Driver/Tools.cpp
+++ lib/Driver/Tools.cpp
@@ -2944,12 +2944,15 @@ void Clang::ConstructJob(Compilation &C, const JobAction &JA,
   }
   // -mkernel implies -mstrict-align; don't add the redundant option.
   if (!KernelOrKext) {
-    if (Args.hasArg(options::OPT_mno_unaligned_access)) {
-      CmdArgs.push_back("-backend-option");
-      CmdArgs.push_back("-arm-strict-align");
-    } else if (Args.hasArg(options::OPT_munaligned_access)) {
-      CmdArgs.push_back("-backend-option");
-      CmdArgs.push_back("-arm-no-strict-align");
+    if (Arg *A = Args.getLastArg(options::OPT_mno_unaligned_access,
+                                 options::OPT_munaligned_access)) {
+      if (A->getOption().matches(options::OPT_mno_unaligned_access)) {
+        CmdArgs.push_back("-backend-option");
+        CmdArgs.push_back("-arm-strict-align");
+      } else {
+        CmdArgs.push_back("-backend-option");
+        CmdArgs.push_back("-arm-no-strict-align");
+      }
     }
   }
 
commit 13eba67c368221df63e6ec7fe3686c9c10082995
Author: Jeroen Hofstee <[email protected]>
Date:   Wed Aug 28 20:55:58 2013 +0200

    test for multiple -munaligned-access / -mno-unaligned-access

diff --git test/Driver/arm-alignment.c test/Driver/arm-alignment.c
index 024c46b..dad66ba 100644
--- test/Driver/arm-alignment.c
+++ test/Driver/arm-alignment.c
@@ -1,9 +1,25 @@
 // RUN: %clang -target arm-none-gnueeabi -munaligned-access -### %s 2> %t
 // RUN: FileCheck --check-prefix=CHECK-UNALIGNED < %t %s
 
+// RUN: %clang -target arm-none-gnueeabi -mstrict-align -munaligned-access -### %s 2> %t
+// RUN: FileCheck --check-prefix=CHECK-UNALIGNED < %t %s
+
+// RUN: %clang -target arm-none-gnueeabi -mno-unaligned-access -munaligned-access -### %s 2> %t
+// RUN: FileCheck --check-prefix=CHECK-UNALIGNED < %t %s
+
 // CHECK-UNALIGNED: "-backend-option" "-arm-no-strict-align"
 
+
 // RUN: %clang -target arm-none-gnueeabi -mno-unaligned-access -### %s 2> %t
 // RUN: FileCheck --check-prefix=CHECK-ALIGNED < %t %s
 
+// RUN: %clang -target arm-none-gnueeabi -mstrict-align -### %s 2> %t
+// RUN: FileCheck --check-prefix=CHECK-ALIGNED < %t %s
+
+// RUN: %clang -target arm-none-gnueabi -munaligned-access -mno-unaligned-access -### %s 2> %t
+// RUN: FileCheck --check-prefix=CHECK-ALIGNED < %t %s
+
+// RUN: %clang -target arm-none-gnueabi -munaligned-access -mstrict-align -### %s 2> %t
+// RUN: FileCheck --check-prefix=CHECK-ALIGNED < %t %s
+
 // CHECK-ALIGNED: "-backend-option" "-arm-strict-align"
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

Reply via email to