[PATCH] D103184: [AArch64] handle -Wa,-march=

jcai19 updated this revision to Diff 353440. jcai19 added a comment. After chatting with @nickdesaulniers, we have decided to abondoned D104656 in favor of keeping all the updates to this differential review for consistency. Appologies for any inconvenience

[PATCH] D103184: [AArch64] handle -Wa,-march=

nickdesaulniers requested changes to this revision. nickdesaulniers added a comment. This revision now requires changes to proceed. removing LGTM until the reland is posted for review here so we can use the history tab to observe the changes. Repository: rG LLVM Github Monorepo CHANGES

[PATCH] D103184: [AArch64] handle -Wa,-march=

jcai19 updated this revision to Diff 353427. jcai19 added a comment. Use the original flawed code for future reference. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D103184/new/ https://reviews.llvm.org/D103184 Files:

[PATCH] D103184: [AArch64] handle -Wa,-march=

jcai19 closed this revision. jcai19 added a comment. Sorry it's probably clearer to use a new differential review for relanding. I've created https://reviews.llvm.org/D104656. Sorry for the noises. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D103184: [AArch64] handle -Wa,-march=

jcai19 updated this revision to Diff 353410. jcai19 edited the summary of this revision. jcai19 added a comment. The original implementation was indeed flawed. The failed test does not specify target triple so on X86 machines the issue won't happen unless I explicitly specify "-target aarch64".

[PATCH] D103184: [AArch64] handle -Wa,-march=

nickdesaulniers added a comment. In D103184#2813748 , @thakis wrote: > In D103184#2804241 , @jcai19 wrote: > >> In D103184#2803768 , @thakis wrote: >> >>> FWIW the

[PATCH] D103184: [AArch64] handle -Wa,-march=

thakis added a comment. In D103184#2804241 , @jcai19 wrote: > In D103184#2803768 , @thakis wrote: > >> FWIW the failure goes away locally if I revert this change here, so it's >> definitely due to this change

[PATCH] D103184: [AArch64] handle -Wa,-march=

jcai19 added a comment. In D103184#2803768 , @thakis wrote: > FWIW the failure goes away locally if I revert this change here, so it's > definitely due to this change here. > > Things have been red for a while, probably good to revert while you >

[PATCH] D103184: [AArch64] handle -Wa,-march=

thakis added a comment. FWIW the failure goes away locally if I revert this change here, so it's definitely due to this change here. Things have been red for a while, probably good to revert while you investigate by this point. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D103184: [AArch64] handle -Wa,-march=

thakis added a comment. In D103184#2803691 , @nickdesaulniers wrote: > In D103184#2803513 , @thakis wrote: > >> This breaks tests on arm macs: http://45.33.8.238/macm1/10931/step_7.txt > > > >>> clang: error:

[PATCH] D103184: [AArch64] handle -Wa,-march=

nickdesaulniers added a comment. In D103184#2803513 , @thakis wrote: > This breaks tests on arm macs: http://45.33.8.238/macm1/10931/step_7.txt >> clang: error: the clang compiler does not support '-Wa,--version' Wouldn't that be

[PATCH] D103184: [AArch64] handle -Wa,-march=

jcai19 added a comment. In D103184#2803513 , @thakis wrote: > This breaks tests on arm macs: http://45.33.8.238/macm1/10931/step_7.txt > > Please take a look and revert for now if it takes a while to fix. Thanks, will address the breakage ASAP.

[PATCH] D103184: [AArch64] handle -Wa,-march=

thakis added a comment. This breaks tests on arm macs: http://45.33.8.238/macm1/10931/step_7.txt Please take a look and revert for now if it takes a while to fix. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D103184/new/

[PATCH] D103184: [AArch64] handle -Wa,-march=

This revision was automatically updated to reflect the committed changes. Closed by commit rGfd11a26d368c: [AArch64] handle -Wa,-march= (authored by jcai19). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D103184/new/ https://reviews.llvm.org/D103184

[PATCH] D103184: [AArch64] handle -Wa,-march=

DavidSpickett accepted this revision. DavidSpickett added a comment. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D103184/new/ https://reviews.llvm.org/D103184 ___ cfe-commits mailing list

[PATCH] D103184: [AArch64] handle -Wa,-march=

jcai19 added a comment. In D103184#2799732 , @nickdesaulniers wrote: > That looks better, thanks @jcai19 . Based on @DavidSpickett 's LGTM I think > this is now ready to land. > > @DavidSpickett did ask: > >> Also please include in the commit message

[PATCH] D103184: [AArch64] handle -Wa,-march=

nickdesaulniers accepted this revision. nickdesaulniers added a comment. This revision is now accepted and ready to land. That looks better, thanks @jcai19 . Based on @DavidSpickett 's LGTM I think this is now ready to land. @DavidSpickett did ask: > Also please include in the commit message

[PATCH] D103184: [AArch64] handle -Wa,-march=

jcai19 updated this revision to Diff 349938. jcai19 marked an inline comment as done. jcai19 added a comment. Update test cases based on comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D103184/new/ https://reviews.llvm.org/D103184 Files:

[PATCH] D103184: [AArch64] handle -Wa,-march=

nickdesaulniers requested changes to this revision. nickdesaulniers added inline comments. This revision now requires changes to proceed. Comment at: clang/test/Driver/aarch64-target-as-march.s:29 +// RUN: FileCheck --check-prefix=MULTIPLE-VALUES %s + +// MULTIPLE-VALUES:

[PATCH] D103184: [AArch64] handle -Wa,-march=

jcai19 added a comment. In D103184#2798398 , @DavidSpickett wrote: >> LGTM; any additional thoughts @DavidSpickett ? > > A couple of additional tests just to be safe but otherwise LGTM. > > Also please include in the commit message the "rules" that

[PATCH] D103184: [AArch64] handle -Wa,-march=

jcai19 updated this revision to Diff 349912. jcai19 added a comment. Address the comments and add more tests. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D103184/new/ https://reviews.llvm.org/D103184 Files:

[PATCH] D103184: [AArch64] handle -Wa,-march=

DavidSpickett accepted this revision. DavidSpickett added a comment. > LGTM; any additional thoughts @DavidSpickett ? A couple of additional tests just to be safe but otherwise LGTM. Comment at: clang/test/Driver/aarch64-target-as-march.s:29 +// RUN: FileCheck

[PATCH] D103184: [AArch64] handle -Wa,-march=

nickdesaulniers accepted this revision. nickdesaulniers added a comment. This revision is now accepted and ready to land. LGTM; any additional thoughts @DavidSpickett ? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D103184/new/

[PATCH] D103184: [AArch64] handle -Wa,-march=

jcai19 updated this revision to Diff 349450. jcai19 marked an inline comment as done. jcai19 added a comment. Add a test case. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D103184/new/ https://reviews.llvm.org/D103184 Files:

[PATCH] D103184: [AArch64] handle -Wa,-march=

nickdesaulniers added inline comments. Comment at: clang/test/Driver/aarch64-target-as-march.s:20-25 +/// No unused argument warnings when there are multiple values +// RUN: %clang --target=aarch64-linux-gnueabi -### -c -Wa,-march=armv8.1-a -Wa,-march=armv8.2-a %s 2>&1 | \ +//

[PATCH] D103184: [AArch64] handle -Wa,-march=

jcai19 marked an inline comment as done. jcai19 added a comment. In D103184#2793055 , @DavidSpickett wrote: > $ cat /tmp/test.s > irg x0, x0 > $ aarch64-unknown-linux-gnu-as -march=armv8.5-a+memtag -march=armv8.1-a > /tmp/test.s -o /dev/null >

[PATCH] D103184: [AArch64] handle -Wa,-march=

DavidSpickett added a comment. $ cat /tmp/test.s irg x0, x0 $ aarch64-unknown-linux-gnu-as -march=armv8.5-a+memtag -march=armv8.1-a /tmp/test.s -o /dev/null /tmp/test.s: Assembler messages: /tmp/test.s:1: Error: selected processor does not support `irg x0,x0' GAS is also taking the

[PATCH] D103184: [AArch64] handle -Wa,-march=

DavidSpickett added a comment. > I understand it's a little bit confusing here, but I was simply trying to > match GCC's behavior (please see the example in my last comment) unless I > misunderstood its output. I definitely agree having consistent behaviors > between Arm and Aarch64 in Clang

[PATCH] D103184: [AArch64] handle -Wa,-march=

jcai19 marked an inline comment as done. jcai19 added inline comments. Comment at: clang/test/Driver/aarch64-target-as-march.s:4 + +/// The last -march wins, and is handled before -Wa,march. All the values of -Wa,march options are added. +// RUN: %clang

[PATCH] D103184: [AArch64] handle -Wa,-march=

jcai19 updated this revision to Diff 349150. jcai19 added a comment. Limit the change to assembler files, and add more tests. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D103184/new/ https://reviews.llvm.org/D103184 Files:

[PATCH] D103184: [AArch64] handle -Wa,-march=

nickdesaulniers added inline comments. Comment at: clang/test/Driver/aarch64-target-as-march.s:4 + +/// The last -march wins, and is handled before -Wa,march. All the values of -Wa,march options are added. +// RUN: %clang --target=aarch64-linux-gnueabi -### -c

[PATCH] D103184: [AArch64] handle -Wa,-march=

jcai19 added a comment. Thanks for the comments. Please see my replies inlined. In D103184#2790486 , @DavidSpickett wrote: > Thanks for taking this up! I never got the time for it. > > I'd like to see the test check that there are no unused argument

[PATCH] D103184: [AArch64] handle -Wa,-march=

DavidSpickett added a comment. Thanks for taking this up! I never got the time for it. I'd like to see the test check that there are no unused argument warnings when there are multiple values. I missed that with another patch in this area. Beyond that I'm not sure the logic is right here.

[PATCH] D103184: [AArch64] handle -Wa,-march=

jcai19 added inline comments. Comment at: clang/lib/Driver/ToolChains/Arch/AArch64.cpp:217 + // after -march. And while only using the the value of last -march, it + // includes all the options passed via -Wa,-march. + success = true; joerg wrote: > This

[PATCH] D103184: [AArch64] handle -Wa,-march=

joerg added inline comments. Comment at: clang/lib/Driver/ToolChains/Arch/AArch64.cpp:217 + // after -march. And while only using the the value of last -march, it + // includes all the options passed via -Wa,-march. + success = true; This comment is

[PATCH] D103184: [AArch64] handle -Wa,-march=

jcai19 created this revision. Herald added subscribers: danielkiss, kristof.beyls. jcai19 requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. This fixed PR#48894 for AArch64. The issue has been fixed for Arm in https://reviews.llvm.org/D95872