[PATCH] D149027: [X86] Add peephole to convert `(Cmp Op32/Op64, Imm8)` -> `(Cmp Op16/Op8, Imm8)`

Sun Apr 23 18:01:28 PDT 2023

pengfei added inline comments.

================
Comment at: llvm/lib/Target/X86/X86ISelDAGToDAG.cpp:5665
+        SDValue TruncN1 = CurDAG->getConstant(
+            C.trunc(NewVT.getScalarSizeInBits()).getZExtValue(), dl, MVT::i32);
+        insertDAGNode(*CurDAG, SDValue(Node, 0), TruncN1);
----------------
craig.topper wrote:
> goldstein.w.n wrote:
> > craig.topper wrote:
> > > Shouldn't the type for the constant be NewVT?
> > Was doing that originally. Was running into:
> > ```
> > MachineOperand.h:370: llvm::Register llvm::MachineOperand::getReg() const: Assertion `isReg() && "This is not a register operand!"' failed.
> > ```
> > 
> > Phoebe suggested this.
> The types of the 2 arguments to X86ISD::CMP are supposed to be the same so there's some other issue going on.
The problem was getConstant vs. getTargetConstant. I tried i8 vs. i32 before, but forgot to change it back. Seems the type here doesn't matter.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149027