[PATCH] D137269: [Clang][AArch64][Darwin] Enable GlobalISel by default for Darwin ARM64 platforms.

thakis added a comment. In D137269#3925163 , @thakis wrote: > Heads-up: This very visibly broke web page rendering in Chromium (but only > when using LTO) (https://crbug.com/1383873) . Not clear yet if this exposed > UB on our end or not. After

[PATCH] D137269: [Clang][AArch64][Darwin] Enable GlobalISel by default for Darwin ARM64 platforms.

thakis added a comment. > One thing that we did notice is that adding `-fno-global-isel` to cflags and > ldflags only had an effect when manually blowing away the thinlto cache. > Maybe whatever decides when to invalidate the thinlto cache needs to learn to > invalidate it when the

[PATCH] D137269: [Clang][AArch64][Darwin] Enable GlobalISel by default for Darwin ARM64 platforms.

thakis added a comment. Heads-up: This very visibly broke web page rendering in Chromium (but only when using LTO) (https://crbug.com/1383873) . Not clear yet if this exposed UB on our end or not. One thing that we did notice is that adding `-fno-global-isel` to cflags and ldflags only had an

[PATCH] D137269: [Clang][AArch64][Darwin] Enable GlobalISel by default for Darwin ARM64 platforms.

aemerson added a comment. Sorry, for some reason I hadn’t received any emails about this so I only noticed now. I’ll look into the issues and come back once the issues are resolved. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137269/new/

[PATCH] D137269: [Clang][AArch64][Darwin] Enable GlobalISel by default for Darwin ARM64 platforms.

aeubanks added a comment. In D137269#3916916 , @nikic wrote: > I'd like to object to enabling this via the frontend. This means that > non-clang frontends will now use a non-default configuration that is not > extensively tested by upstream anymore. >

[PATCH] D137269: [Clang][AArch64][Darwin] Enable GlobalISel by default for Darwin ARM64 platforms.

hans added a comment. Looks like there are a few issues flagged already. Is someone reverting, or what's the plan? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137269/new/ https://reviews.llvm.org/D137269

[PATCH] D137269: [Clang][AArch64][Darwin] Enable GlobalISel by default for Darwin ARM64 platforms.

arsenm added a comment. In D137269#3916911 , @fhahn wrote: > It looks like GISel crashes when building `llvm-test-suite` with -O3 on > ARM64. If it isn't trivial to fix we should probably revert the patch to > bring things back to green. > >

[PATCH] D137269: [Clang][AArch64][Darwin] Enable GlobalISel by default for Darwin ARM64 platforms.

abrachet added a comment. I believe this patch might be the cause of some assertion failures we are seeing in a two stage lto build, as it's only happening for arm64-darwin. See https://ci.chromium.org/ui/p/fuchsia/builders/prod/clang-mac-xarm64/b8798097341668696785/overview and it's

[PATCH] D137269: [Clang][AArch64][Darwin] Enable GlobalISel by default for Darwin ARM64 platforms.

nikic added a comment. I'd like to object to enabling this via the frontend. This means that non-clang frontends will now use a non-default configuration that is not extensively tested by upstream anymore. If you don't want to change tests, you can adjust the RUN lines to explicitly disable

[PATCH] D137269: [Clang][AArch64][Darwin] Enable GlobalISel by default for Darwin ARM64 platforms.

fhahn added a comment. It looks like GISel crashes when building `llvm-test-suite` with -O3 on ARM64. If it isn't trivial to fix we should probably revert the patch to bring things back to green. Reproducer: ; llc -global-isel -O3 target datalayout = "e-m:o-i64:64-i128:128-n32:64-S128"

[PATCH] D137269: [Clang][AArch64][Darwin] Enable GlobalISel by default for Darwin ARM64 platforms.

This revision was automatically updated to reflect the committed changes. Closed by commit rGf64802e8d3e9: [Clang][AArch64][Darwin] Enable GlobalISel by default for Darwin ARM64… (authored by aemerson). Changed prior to commit: https://reviews.llvm.org/D137269?vs=472753=473807#toc Repository:

[PATCH] D137269: [Clang][AArch64][Darwin] Enable GlobalISel by default for Darwin ARM64 platforms.

paquette accepted this revision. paquette added a comment. This revision is now accepted and ready to land. If there's nothing else to add wrt telling people what's changed, I think this looks good? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D137269: [Clang][AArch64][Darwin] Enable GlobalISel by default for Darwin ARM64 platforms.

aemerson updated this revision to Diff 472753. aemerson added a comment. Add a clang release note. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137269/new/ https://reviews.llvm.org/D137269 Files: clang/docs/ReleaseNotes.rst

[PATCH] D137269: [Clang][AArch64][Darwin] Enable GlobalISel by default for Darwin ARM64 platforms.

tschuett added a comment. A release note? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137269/new/ https://reviews.llvm.org/D137269 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D137269: [Clang][AArch64][Darwin] Enable GlobalISel by default for Darwin ARM64 platforms.

aemerson added inline comments. Comment at: clang/lib/Driver/ToolChains/Clang.cpp:7218-7219 + CmdArgs.push_back("-global-isel=1"); + CmdArgs.push_back("-mllvm"); + CmdArgs.push_back("-global-isel-abort=0"); +} arsenm wrote: > Why abort=0? I

[PATCH] D137269: [Clang][AArch64][Darwin] Enable GlobalISel by default for Darwin ARM64 platforms.

arsenm added inline comments. Comment at: clang/lib/Driver/ToolChains/Darwin.cpp:388 + options::OPT_fno_global_isel); +if (!A || !A->getOption().matches(options::OPT_fno_global_isel)) { CmdArgs.push_back("-mllvm"); Actually

[PATCH] D137269: [Clang][AArch64][Darwin] Enable GlobalISel by default for Darwin ARM64 platforms.

arsenm added inline comments. Comment at: clang/lib/Driver/ToolChains/Clang.cpp:7218-7219 + CmdArgs.push_back("-global-isel=1"); + CmdArgs.push_back("-mllvm"); + CmdArgs.push_back("-global-isel-abort=0"); +} Why abort=0? I can understand

[PATCH] D137269: [Clang][AArch64][Darwin] Enable GlobalISel by default for Darwin ARM64 platforms.

paquette added a comment. maybe link back to the discourse thread in the commit message? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137269/new/ https://reviews.llvm.org/D137269 ___ cfe-commits

[PATCH] D137269: [Clang][AArch64][Darwin] Enable GlobalISel by default for Darwin ARM64 platforms.

aemerson created this revision. aemerson added reviewers: paquette, Gerolf, ab, t.p.northover, arsenm. aemerson added a project: LLVM. Herald added a subscriber: kristof.beyls. Herald added a project: All. aemerson requested review of this revision. Herald added subscribers: MaskRay, wdng. Herald