[PATCH] D145299: [X86] Generate better code for std::bit_ceil

[Noah Goldstein via Phabricator via llvm-commits]

goldstein.w.n added inline comments.


================
Comment at: llvm/test/CodeGen/X86/bit_ceil.ll:36
-; CHECK-NEXT:    cmpq $2, %rdi
-; CHECK-NEXT:    cmovbq %rcx, %rax
 ; CHECK-NEXT:    retq
----------------
kazu wrote:
> goldstein.w.n wrote:
> > Is the select at the end just unneeded? If so this would probably be better addressed in the IR or `DAGCombiner` as its a pretty generic. Maybe `DAGCombiner::VisitSETCC`? 
> Correct, we do not need the `select` at the end.
> 
> We do rely on a subtle property that `shlx` masks off the shift count with 31 or 63, depending on the operand size.  I generate `ISD::AND` with 31 (or 63), expecting that the hardware shift instruction absorbs it.
> 
> At the LLVM IR level, the result of the shift instruction is undefined because the final `select` picks 1 for inputs < 2.
> 
> Yes, we could do this in the IR or DAG combiner somewhere.  Let me look into it.
> Correct, we do not need the `select` at the end.
> 
> We do rely on a subtle property that `shlx` masks off the shift count with 31 or 63, depending on the operand size.  I generate `ISD::AND` with 31 (or 63), expecting that the hardware shift instruction absorbs it.
> 
You can still generate the ISD::AND in DAGCombiner, it will be removed in later lowering.




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145299