[PATCH] D65226: [Strict FP] Allow custom operation actions

[Ulrich Weigand via Phabricator via llvm-commits]

uweigand added a comment.

In D65226#1601309 <https://reviews.llvm.org/D65226#1601309>, @cameron.mcinally wrote:

> In D65226#1600831 <https://reviews.llvm.org/D65226#1600831>, @uweigand wrote:
>
> > In D65226#1600009 <https://reviews.llvm.org/D65226#1600009>, @cameron.mcinally wrote:
> >
> > > In D65226#1599743 <https://reviews.llvm.org/D65226#1599743>, @kpn wrote:
> > >
> > > > It looks like we're unrolling some vectors where before we weren't. That seems unfortunate. Is that the reason for the generated code quality regressions?
> > >
> > >
> > > +1. What's the reason behind the scalarization?
> >
> >
> > Those are cases where the non-strict operation actually is not legal: e.g. the X86 target sets the operation action for FADD and FSUB vector operations to Custom.   The old code simply ignored that and just emitted FADD and FSUB anyway as if they were legal, and only by chance did they match an isel pattern anyway.
> >
> > The target can get the old behavior back by simply handling STRICT_FADD and STRICT_FSUB directly (presumably also via Custom operations similar to ones it uses for FADD/FSUB), so this is only a "regression" for the current fallback implementation.
>
>
> Ah, ok. So I think the x86 case is a little more subtle than just *not legal*. The backend checks for *some* Custom lowerings (horizontal ops), but ultimately the non-strict vector FADD/FSUB are legal on x86. The Custom lowering code just returns the original op.


Yes, exactly.  But there's no way common code can know that this is what the Custom lowering code does; in general, it is not OK to just pass an opcode to isel if the target classifies the op as Custom.  But as I said, once the target actually handles the STRICT_ opcodes (either Custom or maybe even Legal, if that's what the target wants), then any regression will go away.  (And as long as the target *doesn't* handle them, they don't really implement the strict semantics anyway and shouldn't be used for anything "real" anyway.)


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D65226/new/

https://reviews.llvm.org/D65226