[PATCH] D153430: [Clang][Driver] Warn on invalid Arm or AArch64 baremetal target triple

2023-06-28 Thread Michael Platings via Phabricator via cfe-commits 
michaelplatings added a comment.

In D153430#4453908 , @MaskRay wrote:

> Thank you for implementing this warning. Side note: it would be better to (a) 
> fix the tests separately from (b) implementing the warning and (c) adding 
> tests to demonstrate the warning.
> This way the patch is more focused.

Agreed, it would have been better to fix the tests first in a separate patch.

In D153430#4454371 , @alexfh wrote:

> After the patch, LLVM still has a number of `aarch64-arm-none-eabi` usages in 
> tests, which is also considered invalid now: 
> https://gcc.godbolt.org/z/z8cY5j68M

Huh. I have no idea why they didn't come up in my find-and-replace. Thanks 
@MaskRay for fixing. I found some more: D153943 



Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D153430/new/

https://reviews.llvm.org/D153430

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D153430: [Clang][Driver] Warn on invalid Arm or AArch64 baremetal target triple

2023-06-27 Thread Alexander Kornienko via Phabricator via cfe-commits 
alexfh added a comment.

After the patch, LLVM still has a number of `aarch64-arm-none-eabi` usages in 
tests, which is also considered invalid now: https://gcc.godbolt.org/z/z8cY5j68M


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D153430/new/

https://reviews.llvm.org/D153430

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D153430: [Clang][Driver] Warn on invalid Arm or AArch64 baremetal target triple

2023-06-27 Thread Fangrui Song via Phabricator via cfe-commits 
MaskRay added a comment.

Thank you for implementing this warning. Side note: it would be better to (a) 
fix the tests separately from (b) implementing the warning and (c) adding tests 
to demonstrate the warning.
This way the patch is more focused.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D153430/new/

https://reviews.llvm.org/D153430

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D153430: [Clang][Driver] Warn on invalid Arm or AArch64 baremetal target triple

2023-06-23 Thread Michael Platings via Phabricator via cfe-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rG041ffc155fd7: [Clang][Driver] Warn on invalid Arm or AArch64 
baremetal target triple (authored by michaelplatings).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D153430/new/

https://reviews.llvm.org/D153430

Files:
  clang/include/clang/Basic/DiagnosticCommonKinds.td
  clang/lib/Driver/Driver.cpp
  clang/test/CodeGen/aapcs64-align.cpp
  clang/test/CodeGen/aarch64-sign-return-address.c
  clang/test/CodeGen/arm_acle.c
  clang/test/CodeGenCXX/float16-declarations.cpp
  clang/test/Driver/aarch64-cssc.c
  clang/test/Driver/aarch64-d128.c
  clang/test/Driver/aarch64-hbc.c
  clang/test/Driver/aarch64-ite.c
  clang/test/Driver/aarch64-lrcpc3.c
  clang/test/Driver/aarch64-ls64.c
  clang/test/Driver/aarch64-lse128.c
  clang/test/Driver/aarch64-mops.c
  clang/test/Driver/aarch64-mte.c
  clang/test/Driver/aarch64-perfmon.c
  clang/test/Driver/aarch64-predres.c
  clang/test/Driver/aarch64-rand.c
  clang/test/Driver/aarch64-ras.c
  clang/test/Driver/aarch64-rdm.c
  clang/test/Driver/aarch64-ssbs.c
  clang/test/Driver/aarch64-the.c
  clang/test/Driver/arm-matrix-multiply.c
  clang/test/Driver/arm-sb.c
  clang/test/Driver/constructors.c
  clang/test/Driver/unsupported-target-arch.c
  clang/test/Frontend/gnu-mcount.c
  clang/test/Headers/arm-fp16-header.c
  clang/test/Headers/arm-neon-header.c
  clang/test/Preprocessor/aarch64-target-features.c
  llvm/test/CodeGen/AArch64/andcompare.ll
  llvm/test/CodeGen/AArch64/andorbrcompare.ll
  llvm/test/CodeGen/AArch64/blockaddress.ll
  llvm/test/CodeGen/AArch64/extern-weak.ll
  llvm/test/CodeGen/AArch64/init-array.ll
  llvm/test/CodeGen/AArch64/literal_pools_float.ll
  llvm/test/CodeGen/AArch64/neg-selects.ll
  llvm/test/CodeGen/AArch64/qmovn.ll
  llvm/test/CodeGen/AArch64/select-constant-xor.ll
  llvm/test/MC/AArch64/armv8.9a-clrbhb.s

Index: llvm/test/MC/AArch64/armv8.9a-clrbhb.s
===
--- llvm/test/MC/AArch64/armv8.9a-clrbhb.s
+++ llvm/test/MC/AArch64/armv8.9a-clrbhb.s
@@ -2,37 +2,37 @@
 // Assembly is always permitted for instructions in the hint space.
 
 // Optional, off by default
-// RUN: llvm-mc -show-encoding -triple aarch64-none-none-eabi < %s | FileCheck %s --check-prefix=HINT_22
-// RUN: llvm-mc -show-encoding -triple aarch64-none-none-eabi -mattr=+v8a < %s | FileCheck %s --check-prefix=HINT_22
-// RUN: llvm-mc -show-encoding -triple aarch64-none-none-eabi -mattr=+v8.8a < %s | FileCheck %s --check-prefix=HINT_22
-// RUN: llvm-mc -show-encoding -triple aarch64-none-none-eabi -mattr=+v9a < %s | FileCheck %s --check-prefix=HINT_22
-// RUN: llvm-mc -show-encoding -triple aarch64-none-none-eabi -mattr=+v9.3a < %s | FileCheck %s --check-prefix=HINT_22
+// RUN: llvm-mc -show-encoding -triple aarch64-none-elf < %s | FileCheck %s --check-prefix=HINT_22
+// RUN: llvm-mc -show-encoding -triple aarch64-none-elf -mattr=+v8a < %s | FileCheck %s --check-prefix=HINT_22
+// RUN: llvm-mc -show-encoding -triple aarch64-none-elf -mattr=+v8.8a < %s | FileCheck %s --check-prefix=HINT_22
+// RUN: llvm-mc -show-encoding -triple aarch64-none-elf -mattr=+v9a < %s | FileCheck %s --check-prefix=HINT_22
+// RUN: llvm-mc -show-encoding -triple aarch64-none-elf -mattr=+v9.3a < %s | FileCheck %s --check-prefix=HINT_22
 
 // Optional, off by default, doubly disabled
-// RUN: llvm-mc -show-encoding -triple aarch64-none-none-eabi -mattr=-clrbhb < %s | FileCheck %s --check-prefix=HINT_22
-// RUN: llvm-mc -show-encoding -triple aarch64-none-none-eabi -mattr=+v8a,-clrbhb < %s | FileCheck %s --check-prefix=HINT_22
-// RUN: llvm-mc -show-encoding -triple aarch64-none-none-eabi -mattr=+v8.8a,-clrbhb < %s | FileCheck %s --check-prefix=HINT_22
-// RUN: llvm-mc -show-encoding -triple aarch64-none-none-eabi -mattr=+v9a,-clrbhb < %s | FileCheck %s --check-prefix=HINT_22
-// RUN: llvm-mc -show-encoding -triple aarch64-none-none-eabi -mattr=+v9.3a,-clrbhb < %s | FileCheck %s --check-prefix=HINT_22
+// RUN: llvm-mc -show-encoding -triple aarch64-none-elf -mattr=-clrbhb < %s | FileCheck %s --check-prefix=HINT_22
+// RUN: llvm-mc -show-encoding -triple aarch64-none-elf -mattr=+v8a,-clrbhb < %s | FileCheck %s --check-prefix=HINT_22
+// RUN: llvm-mc -show-encoding -triple aarch64-none-elf -mattr=+v8.8a,-clrbhb < %s | FileCheck %s --check-prefix=HINT_22
+// RUN: llvm-mc -show-encoding -triple aarch64-none-elf -mattr=+v9a,-clrbhb < %s | FileCheck %s --check-prefix=HINT_22
+// RUN: llvm-mc -show-encoding -triple aarch64-none-elf -mattr=+v9.3a,-clrbhb < %s | FileCheck %s --check-prefix=HINT_22
 
 // Optional, off by default, manually enabled
-// RUN: llvm-mc -show-encoding -triple aarch64-none-none-eabi -mattr=+clrbhb < %s | FileCheck %s --check-prefix=CLRBHB
-// RUN: llvm-mc -show-encoding -triple aarch64-none-none-eabi -mattr=+v8a,+clrbhb < %s | FileCheck %s --check-prefix=CLRBHB
-// RUN: llvm-mc -show-encoding -triple

[PATCH] D153430: [Clang][Driver] Warn on invalid Arm or AArch64 baremetal target triple

2023-06-23 Thread David Spickett via Phabricator via cfe-commits 
DavidSpickett accepted this revision.
DavidSpickett added a comment.

LGTM


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D153430/new/

https://reviews.llvm.org/D153430

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D153430: [Clang][Driver] Warn on invalid Arm or AArch64 baremetal target triple

2023-06-23 Thread Peter Smith via Phabricator via cfe-commits 
peter.smith accepted this revision.
peter.smith added a comment.
This revision is now accepted and ready to land.

Thanks for the update! LGTM.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D153430/new/

https://reviews.llvm.org/D153430

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D153430: [Clang][Driver] Warn on invalid Arm or AArch64 baremetal target triple

2023-06-23 Thread Michael Platings via Phabricator via cfe-commits 
michaelplatings updated this revision to Diff 533904.
michaelplatings added a comment.

Tweak warning message


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D153430/new/

https://reviews.llvm.org/D153430

Files:
  clang/include/clang/Basic/DiagnosticCommonKinds.td
  clang/lib/Driver/Driver.cpp
  clang/test/CodeGen/aapcs64-align.cpp
  clang/test/CodeGen/aarch64-sign-return-address.c
  clang/test/CodeGen/arm_acle.c
  clang/test/CodeGenCXX/float16-declarations.cpp
  clang/test/Driver/aarch64-cssc.c
  clang/test/Driver/aarch64-d128.c
  clang/test/Driver/aarch64-hbc.c
  clang/test/Driver/aarch64-ite.c
  clang/test/Driver/aarch64-lrcpc3.c
  clang/test/Driver/aarch64-ls64.c
  clang/test/Driver/aarch64-lse128.c
  clang/test/Driver/aarch64-mops.c
  clang/test/Driver/aarch64-mte.c
  clang/test/Driver/aarch64-perfmon.c
  clang/test/Driver/aarch64-predres.c
  clang/test/Driver/aarch64-rand.c
  clang/test/Driver/aarch64-ras.c
  clang/test/Driver/aarch64-rdm.c
  clang/test/Driver/aarch64-ssbs.c
  clang/test/Driver/aarch64-the.c
  clang/test/Driver/arm-matrix-multiply.c
  clang/test/Driver/arm-sb.c
  clang/test/Driver/constructors.c
  clang/test/Driver/unsupported-target-arch.c
  clang/test/Frontend/gnu-mcount.c
  clang/test/Headers/arm-fp16-header.c
  clang/test/Headers/arm-neon-header.c
  clang/test/Preprocessor/aarch64-target-features.c
  llvm/test/CodeGen/AArch64/andcompare.ll
  llvm/test/CodeGen/AArch64/andorbrcompare.ll
  llvm/test/CodeGen/AArch64/blockaddress.ll
  llvm/test/CodeGen/AArch64/extern-weak.ll
  llvm/test/CodeGen/AArch64/init-array.ll
  llvm/test/CodeGen/AArch64/literal_pools_float.ll
  llvm/test/CodeGen/AArch64/neg-selects.ll
  llvm/test/CodeGen/AArch64/qmovn.ll
  llvm/test/CodeGen/AArch64/select-constant-xor.ll
  llvm/test/MC/AArch64/armv8.9a-clrbhb.s

Index: llvm/test/MC/AArch64/armv8.9a-clrbhb.s
===
--- llvm/test/MC/AArch64/armv8.9a-clrbhb.s
+++ llvm/test/MC/AArch64/armv8.9a-clrbhb.s
@@ -2,37 +2,37 @@
 // Assembly is always permitted for instructions in the hint space.
 
 // Optional, off by default
-// RUN: llvm-mc -show-encoding -triple aarch64-none-none-eabi < %s | FileCheck %s --check-prefix=HINT_22
-// RUN: llvm-mc -show-encoding -triple aarch64-none-none-eabi -mattr=+v8a < %s | FileCheck %s --check-prefix=HINT_22
-// RUN: llvm-mc -show-encoding -triple aarch64-none-none-eabi -mattr=+v8.8a < %s | FileCheck %s --check-prefix=HINT_22
-// RUN: llvm-mc -show-encoding -triple aarch64-none-none-eabi -mattr=+v9a < %s | FileCheck %s --check-prefix=HINT_22
-// RUN: llvm-mc -show-encoding -triple aarch64-none-none-eabi -mattr=+v9.3a < %s | FileCheck %s --check-prefix=HINT_22
+// RUN: llvm-mc -show-encoding -triple aarch64-none-elf < %s | FileCheck %s --check-prefix=HINT_22
+// RUN: llvm-mc -show-encoding -triple aarch64-none-elf -mattr=+v8a < %s | FileCheck %s --check-prefix=HINT_22
+// RUN: llvm-mc -show-encoding -triple aarch64-none-elf -mattr=+v8.8a < %s | FileCheck %s --check-prefix=HINT_22
+// RUN: llvm-mc -show-encoding -triple aarch64-none-elf -mattr=+v9a < %s | FileCheck %s --check-prefix=HINT_22
+// RUN: llvm-mc -show-encoding -triple aarch64-none-elf -mattr=+v9.3a < %s | FileCheck %s --check-prefix=HINT_22
 
 // Optional, off by default, doubly disabled
-// RUN: llvm-mc -show-encoding -triple aarch64-none-none-eabi -mattr=-clrbhb < %s | FileCheck %s --check-prefix=HINT_22
-// RUN: llvm-mc -show-encoding -triple aarch64-none-none-eabi -mattr=+v8a,-clrbhb < %s | FileCheck %s --check-prefix=HINT_22
-// RUN: llvm-mc -show-encoding -triple aarch64-none-none-eabi -mattr=+v8.8a,-clrbhb < %s | FileCheck %s --check-prefix=HINT_22
-// RUN: llvm-mc -show-encoding -triple aarch64-none-none-eabi -mattr=+v9a,-clrbhb < %s | FileCheck %s --check-prefix=HINT_22
-// RUN: llvm-mc -show-encoding -triple aarch64-none-none-eabi -mattr=+v9.3a,-clrbhb < %s | FileCheck %s --check-prefix=HINT_22
+// RUN: llvm-mc -show-encoding -triple aarch64-none-elf -mattr=-clrbhb < %s | FileCheck %s --check-prefix=HINT_22
+// RUN: llvm-mc -show-encoding -triple aarch64-none-elf -mattr=+v8a,-clrbhb < %s | FileCheck %s --check-prefix=HINT_22
+// RUN: llvm-mc -show-encoding -triple aarch64-none-elf -mattr=+v8.8a,-clrbhb < %s | FileCheck %s --check-prefix=HINT_22
+// RUN: llvm-mc -show-encoding -triple aarch64-none-elf -mattr=+v9a,-clrbhb < %s | FileCheck %s --check-prefix=HINT_22
+// RUN: llvm-mc -show-encoding -triple aarch64-none-elf -mattr=+v9.3a,-clrbhb < %s | FileCheck %s --check-prefix=HINT_22
 
 // Optional, off by default, manually enabled
-// RUN: llvm-mc -show-encoding -triple aarch64-none-none-eabi -mattr=+clrbhb < %s | FileCheck %s --check-prefix=CLRBHB
-// RUN: llvm-mc -show-encoding -triple aarch64-none-none-eabi -mattr=+v8a,+clrbhb < %s | FileCheck %s --check-prefix=CLRBHB
-// RUN: llvm-mc -show-encoding -triple aarch64-none-none-eabi -mattr=+v8.8a,+clrbhb < %s | FileCheck %s --check-prefix=CLRBHB
-// RUN:

[PATCH] D153430: [Clang][Driver] Warn on invalid Arm or AArch64 baremetal target triple

2023-06-23 Thread David Spickett via Phabricator via cfe-commits 
DavidSpickett added a comment.

> Does that make it clearer? (I still suspect that people won't understand what 
> is meant by "environment" here but I can't think of a better explanation).

"clang triple environment" into Google gets you the cross compilation guide and 
the Triple class docs so "environment" is useful here as one more keyword for 
that.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D153430/new/

https://reviews.llvm.org/D153430

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D153430: [Clang][Driver] Warn on invalid Arm or AArch64 baremetal target triple

2023-06-22 Thread Peter Smith via Phabricator via cfe-commits 
peter.smith added a comment.

> clang: warning: mismatch between architecture and environment in target 
> triple 'aarch64-none-eabi'; did you mean 'aarch64-none-elf'? 
> [-Winvalid-command-line-argument]

That looks good to me. Would be happy with that.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D153430/new/

https://reviews.llvm.org/D153430

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D153430: [Clang][Driver] Warn on invalid Arm or AArch64 baremetal target triple

2023-06-22 Thread Michael Platings via Phabricator via cfe-commits 
michaelplatings added a comment.

>selects the fallback GCC toolchain driver; did you mean 
> 

This is a correct description of the current behaviour, but I want to keep the 
freedom to change the behaviour in future. Therefore I'd like to avoid defining 
what behaviour an invalid target triple will cause.

> - Why is it invalid?

Fair question. I could rephrase to:

  clang: warning: mismatch between architecture and environment in target 
triple 'aarch64-none-eabi'; did you mean 'aarch64-none-elf'? 
[-Winvalid-command-line-argument]

Does that make it clearer? (I still suspect that people won't understand what 
is meant by "environment" here but I can't think of a better explanation).

> - I assumed it was an error message, and was about to write a comment until I 
> saw it was a warning.

It might not be clear in the code but `warning:` is automatically part of the 
output.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D153430/new/

https://reviews.llvm.org/D153430

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D153430: [Clang][Driver] Warn on invalid Arm or AArch64 baremetal target triple

2023-06-22 Thread Peter Smith via Phabricator via cfe-commits 
peter.smith added a comment.

My initial reactions to seeing Invalid target triple aarch64-none-eabi; try 
aarch64-none-elf [*] were:

- Why is it invalid?
- I assumed it was an error message, and was about to write a comment until I 
saw it was a warning.

[X] now did you mean (thanks for making that change).

An alternative suggestion:

   selects the fallback GCC toolchain driver; did you mean 


This isn't something I'm going to die on a hill for. Perhaps add some more 
reviewers to get some opinions, I'm happy to go with the consensus.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D153430/new/

https://reviews.llvm.org/D153430

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits