shchenz 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:
> 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.
haha, I mean fair....
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D124060/new/
https://reviews.llvm.org/D124060
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
