shchenz added inline comments.
================ Comment at: clang/lib/Basic/Targets/PPC.cpp:519 .Default(false); + Features["crbits"] = llvm::StringSwitch<bool>(CPU) + .Case("ppc64le", true) ---------------- nemanjai wrote: > shchenz wrote: > > amyk wrote: > > > 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. > > hmm, OK, then we still have the same issue that this patch fixes for > > pre-POWER8. I'm ok with leaving it for now as IMO Power7/Power6 should not > > be major versions for PowerPC. > Up until Power8, the only instructions we had that require `crbits` were > CR-logicals (which existed since the POWER architecture - i.e. before > PowerPC). In Power8, we added BCD instructions that also require `crbits`. So > that's why we turn it on for Power8 and up. Fail enough. 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