[PATCH] D124894: Avoid 8 and 16bit switch conditions on x86

[Matthias Braun via Phabricator via llvm-commits]

MatzeB added inline comments.


================
Comment at: llvm/lib/CodeGen/TargetLoweringBase.cpp:1614
+  EVT OldVT = getValueType(DL, CondType);
+  MVT RegType = getRegisterType(Context, OldVT);
+  unsigned RegWidth = RegType.getSizeInBits();
----------------
MatzeB wrote:
> craig.topper wrote:
> > MatzeB wrote:
> > > MatzeB wrote:
> > > > craig.topper wrote:
> > > > > MatzeB wrote:
> > > > > > craig.topper wrote:
> > > > > > > If we just gave targets control over RegType here would that be enough?
> > > > > > The callback is used by `CodeGenPrepare` though which deals with llvm IR and rather has `Type*`s than `MVT`s...
> > > > > Ok could we return ExtType from the target and use ExtType to calculate RegWidth for `if` on 1617?
> > > > > 
> > > > > Realy, I'm wondering why we had to move all of the code into TargetLowering and duplicate Argument attributes checking in X86. Or is there some subtle difference that prevents us from sharing the Argument attribute handling.
> > > > If you prefer we can share the logic starting at line 1632 (deciding between ZExt/SExt and the argument handling) simplifying the X86 callback with the drawback that targets can no longer opt-out of that logic (admittedly I don't know why they would want to opt-out, so I don't care too deeply).
> > > Hmm we need an `MVT` if we want to have the `isSExtCheaperThanZExt` in the shared code as there's no equivalent callback for `Type*`...
> > I guess it might be better to give targets the control. There was a patch proposing to ignore the attributes on X86.  D122963.
> Oh, I just switched it to a more minimal version of this patch. Should I go back to the old version?
> There was a patch proposing to ignore the attributes on X86

FWIW: While working on this I did originally not replicate the logic in the X86 callback and the result was worse at least for the llvm unit-tests.

I think ultimately we had a global instruction selection and wouldn't need to rely on IR->IR transformations upfront... (because I think for optimal results you want to control the overflow-check independently of the value used for the jump-table address calculations; it also depends about what sorts of extensions and truncations the target gets for free; etc.).

But yeah I think for the LLVM of today this patch here at least improves some cases.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124894