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

Reply via email to