[PATCH] D57389: [X86] Improve use of SHLD/SHRD

Amaury SECHET via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Jan 31 17:23:46 PST 2019


deadalnix marked an inline comment as done.
deadalnix added a comment.
Herald added a project: LLVM.

@RKSimon That sound reasonable. The main motivation for this patch is to fix a regression introduced by D57367 <https://reviews.llvm.org/D57367> , so would that be possible to get at least a concept ack on it ?



================
Comment at: lib/Target/X86/X86ISelLowering.cpp:37054
+        ShAmt1Op1 = ShAmt1Op1.getOperand(0);
+      }
       if (ShAmt1Op1.getOpcode() == ISD::TRUNCATE)
----------------
craig.topper wrote:
> deadalnix wrote:
> > RKSimon wrote:
> > > Maybe use DAG.GetDemandedBits ?
> > It's not obvious to me that it would work. The and opcode itself uses it, so there is no need to redo it here. We also need to know if masking took place to know what we can accept as LHS for the sub opcode.
> I know this code (Bits - 1) check is used in other places earlier in this function, but is that valid for i16 SHLD/SHRD? SHLD/SHRD hardware mask shift amount to 5 bits on i16/i32 and 6 bits on i64. 
I do not know if this is valid for i16. If that isn't, this code is bogous already. Would you have a test case I can rely on to do this ?


Repository:
  rL LLVM

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

https://reviews.llvm.org/D57389





More information about the llvm-commits mailing list