amyk marked an inline comment as done. amyk added inline comments.
================ Comment at: clang/lib/Basic/Targets/PPC.cpp:519 .Default(false); + Features["crbits"] = llvm::StringSwitch<bool>(CPU) + .Case("ppc64le", true) ---------------- shchenz wrote: > If we set the `+crbits` by the arch name, do we still need the customization > (Turn on crbits for O2 and above) in `computeFSAdditions()`? Yeah, that's a good point. I looked into this previously, and it appears that addition of `-mcrbits` inside `computeFSAdditions()` may still be necessary. In particular, we have test cases that test pre-POWER8 with optimizations on (or, if no optimization level is provided, then -O2 is assumed the default). In these cases, much of the code changes if the customization inside `computeFSAdditions()` is removed because we would no longer be using crbits pre-P8. ================ Comment at: clang/test/Driver/ppc-crbits.cpp:50 +// RUN: %clang -target powerpc64le-unknown-linux-gnu -mcpu=pwr7 -mno-crbits \ +// RUN: -emit-llvm -S %s -o - | FileCheck %s --check-prefix=HAS-NOCRBITS + ---------------- shchenz wrote: > Do we need some cases for AIX? Thanks, I've added some. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D124060/new/ https://reviews.llvm.org/D124060 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits