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

[Matthias Braun via Phabricator via llvm-commits]

MatzeB marked an inline comment as done.
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();
----------------
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?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124894