[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/lib/Target/X86/X86ISelLowering.cpp:47340
+  if (Add.getOperand(0) != Cond.getOperand(0) ||
+      !isAllOnesConstant(Add.getOperand(1)))
+    return SDValue();
----------------
This all seems a bit a bit brittle, for example if someone does `std::bit_ceil(x + 1)` we won't have the `add` on `ctlz`:

```
  %2 = tail call i32 @llvm.ctlz.i32(i32 %0, i1 false), !range !5
  %3 = sub nuw nsw i32 32, %2
  %4 = shl nuw i32 1, %3
  %5 = add i32 %0, -1
  %6 = icmp ult i32 %5, -2
  %7 = select i1 %6, i32 %4, i32 1
  ret i32 %7
```

is there a way we could more generically detect that the `select` is unneeded?


================
Comment at: llvm/test/CodeGen/X86/bit_ceil.ll:2
 ; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
 ; RUN: llc < %s -mtriple=x86_64-unknown-unknown -mattr=+bmi2,+lzcnt | FileCheck %s
 
----------------
Can you add tests w.o `bmi` and w.o `lzcnt` enabled?


================
Comment at: llvm/test/CodeGen/X86/bit_ceil.ll:36
-; CHECK-NEXT:    cmpq $2, %rdi
-; CHECK-NEXT:    cmovbq %rcx, %rax
 ; CHECK-NEXT:    retq
----------------
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`? 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145299