[PATCH] D154357: [Driver] Recognize powerpc-unknown-eabi as a bare-metal toolchain

2023-07-21 Thread Christian Walther via Phabricator via cfe-commits
cwalther added inline comments. Comment at: clang/test/Driver/baremetal.cpp:348 +// RUN: %clang %s -### --target=powerpc-unknown-eabi 2>&1 \ +// RUN: | FileCheck --check-prefix=CHECK-PPCEABI %s MaskRay wrote: > MaskRay wrote: > > Without a sysroot, we may

[PATCH] D154357: [Driver] Recognize powerpc-unknown-eabi as a bare-metal toolchain

2023-07-20 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/test/Driver/baremetal.cpp:348 +// RUN: %clang %s -### --target=powerpc-unknown-eabi 2>&1 \ +// RUN: | FileCheck --check-prefix=CHECK-PPCEABI %s MaskRay wrote: > Without a sysroot, we may pick up

[PATCH] D154357: [Driver] Recognize powerpc-unknown-eabi as a bare-metal toolchain

2023-07-14 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D154357#4500241 , @cwalther wrote: > Thank you! > > So I guess next time I submit a patch here (nothing planned at the moment), I > should put the proposed commit message into the summary field and the > backstory and start

[PATCH] D154357: [Driver] Recognize powerpc-unknown-eabi as a bare-metal toolchain

2023-07-14 Thread Christian Walther via Phabricator via cfe-commits
cwalther added a comment. Thank you! So I guess next time I submit a patch here (nothing planned at the moment), I should put the proposed commit message into the summary field and the backstory and start of discussion / open questions into a first comment. But maybe the point is moot as I

[PATCH] D154357: [Driver] Recognize powerpc-unknown-eabi as a bare-metal toolchain

2023-07-13 Thread Fangrui Song via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGd986462b5053: [Driver] Recognize powerpc-unknown-eabi as a bare-metal toolchain (authored by cwalther, committed by MaskRay). Repository: rG LLVM

[PATCH] D154357: [Driver] Recognize powerpc-unknown-eabi as a bare-metal toolchain

2023-07-13 Thread Christian Walther via Phabricator via cfe-commits
cwalther added a comment. Christian Walther CHANGES SINCE LAST ACTION https://reviews.llvm.org/D154357/new/ https://reviews.llvm.org/D154357 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D154357: [Driver] Recognize powerpc-unknown-eabi as a bare-metal toolchain

2023-07-13 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D154357#4497973 , @cwalther wrote: > Okay. No, I don’t have write access (as far as I know) and would appreciate > if you (or any other maintainer) could land the patch, with any commit > message you deem appropriate. Happy

[PATCH] D154357: [Driver] Recognize powerpc-unknown-eabi as a bare-metal toolchain

2023-07-13 Thread Christian Walther via Phabricator via cfe-commits
cwalther added a comment. Okay. No, I don’t have write access (as far as I know) and would appreciate if you (or any other maintainer) could land the patch, with any commit message you deem appropriate. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D154357/new/

[PATCH] D154357: [Driver] Recognize powerpc-unknown-eabi as a bare-metal toolchain

2023-07-13 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D154357#4496247 , @cwalther wrote: > That summary wasn’t intended to be a commit message at all, but a description > of the problem and start of the discussion. My commit message was part of the > uploaded patch (since I

[PATCH] D154357: [Driver] Recognize powerpc-unknown-eabi as a bare-metal toolchain

2023-07-13 Thread Christian Walther via Phabricator via cfe-commits
cwalther added a comment. I have gone ahead and removed the last paragraph of the summary, assuming that was what was requested. Happy to make other edits. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D154357/new/ https://reviews.llvm.org/D154357

[PATCH] D154357: [Driver] Recognize powerpc-unknown-eabi as a bare-metal toolchain

2023-07-13 Thread Christian Walther via Phabricator via cfe-commits
cwalther added a comment. That summary wasn’t intended to be a commit message at all, but a description of the problem and start of the discussion. My commit message was part of the uploaded patch (since I generated it using `git show`) and said > [Driver] Recognize powerpc-*-unknown-eabi as a

[PATCH] D154357: [Driver] Recognize powerpc-unknown-eabi as a bare-metal toolchain

2023-07-13 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay accepted this revision. MaskRay added a comment. This revision is now accepted and ready to land. In D154357#4492625 , @cwalther wrote: > I am unable to provoke any problem with my tests by putting various things in > `/usr/lib/gcc/`, and I also

[PATCH] D154357: [Driver] Recognize powerpc-unknown-eabi as a bare-metal toolchain

2023-07-12 Thread Christian Walther via Phabricator via cfe-commits
cwalther added a comment. I am unable to provoke any problem with my tests by putting various things in `/usr/lib/gcc/`, and I also can’t find any code that would go looking there on the code path that is exercised by these tests. Can you give me a hint where that code is, so I can try harder

[PATCH] D154357: [Driver] Recognize powerpc-unknown-eabi as a bare-metal toolchain

2023-07-11 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. The code change looks good but the driver test will cause an issue. I think we need a fake sysroot tree under `Inputs/`. Otherwise if we have a system cross gcc at `/usr/lib/gcc{,-cross}/powerpc64le-unknown-eabi/12`, Clang Driver will pick up these files. This is really

[PATCH] D154357: [Driver] Recognize powerpc-unknown-eabi as a bare-metal toolchain

2023-07-11 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/test/Driver/baremetal.cpp:348 +// RUN: %clang %s -### --target=powerpc-unknown-eabi 2>&1 \ +// RUN: | FileCheck --check-prefix=CHECK-PPCEABI %s Without a sysroot, we may pick up powerpc-unknown-eabi-gcc (if

[PATCH] D154357: [Driver] Recognize powerpc-unknown-eabi as a bare-metal toolchain

2023-07-11 Thread Christian Walther via Phabricator via cfe-commits
cwalther updated this revision to Diff 539132. cwalther added a comment. Oops, forgot to run git-clang-format. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D154357/new/ https://reviews.llvm.org/D154357 Files: clang/lib/Driver/ToolChains/BareMetal.cpp

[PATCH] D154357: [Driver] Recognize powerpc-unknown-eabi as a bare-metal toolchain

2023-07-11 Thread Nemanja Ivanovic via Phabricator via cfe-commits
nemanjai accepted this revision. nemanjai added a comment. No concerns from the perspective of PowerPC here. Of course, my focus is primarily on the server side of things but I am not aware of any other group that could be adversely affected. CHANGES SINCE LAST ACTION

[PATCH] D154357: [Driver] Recognize powerpc-unknown-eabi as a bare-metal toolchain

2023-07-11 Thread Christian Walther via Phabricator via cfe-commits
cwalther updated this revision to Diff 539033. cwalther added a comment. Updated patch according to current discussion: - remove check for vendor, any vendor is accepted - replace cascaded `if` by `&&` - rebase on main a3f9ce6

[PATCH] D154357: [Driver] Recognize powerpc-unknown-eabi as a bare-metal toolchain

2023-07-11 Thread Christian Walther via Phabricator via cfe-commits
cwalther added a comment. > I think `&&` is cleaner: `return Triple.getOS() == llvm::Triple::UnknownOS && > ...`; Totally agree. I was just following precedent there. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D154357/new/

[PATCH] D154357: [Driver] Recognize powerpc-unknown-eabi as a bare-metal toolchain

2023-07-11 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D154357#4487896 , @cwalther wrote: > [...] There is a PowerPC EABI: > https://www.nxp.com/docs/en/application-note/PPCEABI.pdf . I wouldn’t know if > Clang/LLD obey it, but it works for us… Good to know! I read a newer ABI

[PATCH] D154357: [Driver] Recognize powerpc-unknown-eabi as a bare-metal toolchain

2023-07-11 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D154357#4487896 , @cwalther wrote: > Thanks for the comments. Yes, `powerpc-*-eabi` should be valid (sorry if I > was unclear about that – my parenthetical was only about AArch64). There is a > PowerPC EABI:

[PATCH] D154357: [Driver] Recognize powerpc-unknown-eabi as a bare-metal toolchain

2023-07-11 Thread Christian Walther via Phabricator via cfe-commits
cwalther added a comment. Thanks for the comments. Yes, `powerpc-*-eabi` should be valid (sorry if I was unclear about that – my parenthetical was only about AArch64). There is a PowerPC EABI: https://www.nxp.com/docs/en/application-note/PPCEABI.pdf . I wouldn’t know if Clang/LLD obey it, but

[PATCH] D154357: [Driver] Recognize powerpc-unknown-eabi as a bare-metal toolchain

2023-07-10 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D154357#4485881 , @jroelofs wrote: > gcc docs seem to indicate that these are valid triples: > > https://gcc.gnu.org/install/specific.html#powerpc-x-eabi Thanks, good to know. `powerpc-*-eabi` indicates that the eabi

[PATCH] D154357: [Driver] Recognize powerpc-unknown-eabi as a bare-metal toolchain

2023-07-10 Thread Michael Platings via Phabricator via cfe-commits
michaelplatings accepted this revision. michaelplatings added a comment. Hi @MaskRay, thanks for the add. Yes, we've been deleting a lot of `eabi` recently, but that's specific to AArch64. I have no particular insight into PowerPC but from @jroelofs' link, I agree `eabi` seems correct here.

[PATCH] D154357: [Driver] Recognize powerpc-unknown-eabi as a bare-metal toolchain

2023-07-10 Thread Jon Roelofs via Phabricator via cfe-commits
jroelofs added a comment. gcc docs seem to indicate that these are valid triples: https://gcc.gnu.org/install/specific.html#powerpc-x-eabi Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D154357/new/ https://reviews.llvm.org/D154357

[PATCH] D154357: [Driver] Recognize powerpc-unknown-eabi as a bare-metal toolchain

2023-07-10 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. > At Indel, we have been building bare-metal embedded applications that run on > custom PowerPC and ARM systems with Clang and LLD for a couple of years now, > using target triples powerpc-indel-eabi, powerpc-indel-eabi750, > arm-indel-eabi, aarch64-indel-eabi (which I

[PATCH] D154357: [Driver] Recognize powerpc-unknown-eabi as a bare-metal toolchain

2023-07-10 Thread Jon Roelofs via Phabricator via cfe-commits
jroelofs added a comment. I don't think those failures look related. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D154357/new/ https://reviews.llvm.org/D154357 ___ cfe-commits mailing list

[PATCH] D154357: [Driver] Recognize powerpc-unknown-eabi as a bare-metal toolchain

2023-07-04 Thread Christian Walther via Phabricator via cfe-commits
cwalther added a comment. Should I be worried about the build and test failures? At a glance, they look unrelated to my changes, so unless told otherwise I’m going to ignore them. Comment at: clang/lib/Driver/ToolChains/BareMetal.cpp:168 + + if (Triple.getVendor() !=

[PATCH] D154357: [Driver] Recognize powerpc-unknown-eabi as a bare-metal toolchain

2023-07-03 Thread Jon Roelofs via Phabricator via cfe-commits
jroelofs added a comment. > It seems to me that maybe anything with OS none (although that doesn’t seem > to be distinguished from unknown) I considered splitting `none` from `unknown` in Triple, but never had the time to do it. I think this would be good for baremetal toolchains in light of

[PATCH] D154357: [Driver] Recognize powerpc-unknown-eabi as a bare-metal toolchain

2023-07-03 Thread Christian Walther via Phabricator via cfe-commits
cwalther created this revision. cwalther added reviewers: MaskRay, jroelofs. cwalther added projects: clang, PowerPC. Herald added subscribers: luismarques, steven.zhang, s.egerton, shchenz, abidh, PkmX, simoncook, asb, kristof.beyls, arichardson, nemanjai. Herald added a project: All. cwalther