Skip to content

Commit

Permalink
[SelectionDAG] Use int64_t to store the integer power of llvm.powi
Browse files Browse the repository at this point in the history
https://llvm.org/docs/LangRef.html#llvm-powi-intrinsic
The max length of the integer power of `llvm.powi` intrinsic is 32, and
the value can be negative. If we use `int32_t` to store this value, `-Val`
will underflow when it is `INT32_MIN`

The issue was reported in D149033.
  • Loading branch information
KanRobert committed May 2, 2023
1 parent c2dd36c commit 8f966ce
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 1 deletion.
2 changes: 1 addition & 1 deletion llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5451,7 +5451,7 @@ static SDValue ExpandPowI(const SDLoc &DL, SDValue LHS, SDValue RHS,
// it's beneficial on the target, otherwise we end up lowering to a call to
// __powidf2 (for example).
if (ConstantSDNode *RHSC = dyn_cast<ConstantSDNode>(RHS)) {
int Val = RHSC->getSExtValue();
int64_t Val = RHSC->getSExtValue();

// powi(x, 0) -> 1.0
if (Val == 0)
Expand Down
17 changes: 17 additions & 0 deletions llvm/test/CodeGen/X86/powi-int32min.ll
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
; RUN: llc -mtriple=x86_64-unknown-unknown < %s | FileCheck %s

define float @test_powi(ptr %p) nounwind {
; CHECK-LABEL: test_powi:
; CHECK: # %bb.0: # %bb
; CHECK-NEXT: movss {{.*#+}} xmm1 = mem[0],zero,zero,zero
; CHECK-COUNT-31: mulss %xmm1, %xmm1
; CHECK-NEXT: movss {{.*#+}} xmm0 = mem[0],zero,zero,zero
; CHECK-NEXT: divss %xmm1, %xmm0
; CHECK-NEXT: retq
bb:
%load = load float, ptr %p, align 4
%call = call contract float @llvm.powi.f32.i32(float %load, i32 -2147483648)
ret float %call
}

declare float @llvm.powi.f32.i32(float, i32)

3 comments on commit 8f966ce

@cmtice
Copy link
Contributor

@cmtice cmtice commented on 8f966ce May 3, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a heads up: This change/test fails with some sanitizer builds:
exit status 2
third_party/llvm/llvm-project/llvm/include/llvm/CodeGen/TargetLowering.h:2303:18: runtime error: negation of -2147483648 cannot be represented in type 'int'; cast to an unsigned type to negate this value to itself
#0 0x5592c040ab47 in isBeneficialToExpandPowI third_party/llvm/llvm-project/llvm/include/llvm/CodeGen/TargetLowering.h:2303:18
#1 0x5592c040ab47 in ExpandPowI(llvm::SDLoc const&, llvm::SDValue, llvm::SDValue, llvm::SelectionDAG&) third_party/llvm/llvm-project/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp:5460:37
#2 0x5592c04041b0 in llvm::SelectionDAGBuilder::visitIntrinsicCall(llvm::CallInst const&, unsigned int) third_party/llvm/llvm-project/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp:6337:18
#3 0x5592c03cd0ce in llvm::SelectionDAGBuilder::visitCall(llvm::CallInst const&) third_party/llvm/llvm-project/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp:8423:9
#4 0x5592c03ba2c6 in llvm::SelectionDAGBuilder::visit(unsigned int, llvm::User const&) third_party/llvm/llvm-project/llvm/include/llvm/IR/Instruction.def:209:1
#5 0x5592c03b714b in llvm::SelectionDAGBuilder::visit(llvm::Instruction const&) third_party/llvm/llvm-project/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp:1172:3
#6 0x5592c04512e7 in llvm::SelectionDAGISel::SelectBasicBlock(llvm::ilist_iterator<llvm::ilist_detail::node_options<llvm::Instruction, false, false, void>, false, true>, llvm::ilist_iterator<llvm::ilist_detail::node_options<llvm::Instruction, false, false, void>, false, true>, bool&) third_party/llvm/llvm-project/llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp:691:12
#7 0x5592c0450bbf in llvm::SelectionDAGISel::SelectAllBasicBlocks(llvm::Function const&) third_party/llvm/llvm-project/llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp:1673:7
#8 0x5592c044ba14 in llvm::SelectionDAGISel::runOnMachineFunction(llvm::MachineFunction&) third_party/llvm/llvm-project/llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp:482:3
#9 0x5592bf5f6df5 in (anonymous namespace)::X86DAGToDAGISel::runOnMachineFunction(llvm::MachineFunction&) third_party/llvm/llvm-project/llvm/lib/Target/X86/X86ISelDAGToDAG.cpp:191:25
#10 0x5592bfdf0bfa in llvm::MachineFunctionPass::runOnFunction(llvm::Function&) third_party/llvm/llvm-project/llvm/lib/CodeGen/MachineFunctionPass.cpp:91:13
#11 0x5592c1c9a224 in llvm::FPPassManager::runOnFunction(llvm::Function&) third_party/llvm/llvm-project/llvm/lib/IR/LegacyPassManager.cpp:1435:27
#12 0x5592c1ca45f1 in llvm::FPPassManager::runOnModule(llvm::Module&) third_party/llvm/llvm-project/llvm/lib/IR/LegacyPassManager.cpp:1481:16
#13 0x5592c1c9acc0 in runOnModule third_party/llvm/llvm-project/llvm/lib/IR/LegacyPassManager.cpp:1550:27
#14 0x5592c1c9acc0 in llvm::legacy::PassManagerImpl::run(llvm::Module&) third_party/llvm/llvm-project/llvm/lib/IR/LegacyPassManager.cpp:535:44
#15 0x5592c1ca48fa in llvm::legacy::PassManager::run(llvm::Module&) third_party/llvm/llvm-project/llvm/lib/IR/LegacyPassManager.cpp:1677:14
#16 0x5592bd7f8670 in compileModule(char**, llvm::LLVMContext&) third_party/llvm/llvm-project/llvm/tools/llc/llc.cpp:756:8
#17 0x5592bd7f4458 in main third_party/llvm/llvm-project/llvm/tools/llc/llc.cpp:420:22
#18 0x7ff9020a7632 in __libc_start_main (/usr/grte/v5/lib64/libc.so.6+0x61632) (BuildId: 280088eab084c30a3992a9bce5c35b44)
#19 0x5592bd721c69 in _start /build/work/ab393f4ac612f9027aae6b1a7226027ba2a2/google3/blaze-out/k8-opt/bin/third_party/grte/v5_src/grte-scratch/BUILD/src/csu/../sysdeps/x86_64/start.S:120

SUMMARY: UndefinedBehaviorSanitizer: signed-integer-overflow third_party/llvm/llvm-project/llvm/include/llvm/CodeGen/TargetLowering.h:2303:18 in
FileCheck error: '' is empty.
FileCheck command line: /build/work/f43a1db9c1a0389ec7418001d2a110d9a562/google3/runfiles/google3/third_party/llvm/llvm-project/llvm/FileCheck --allow-unused-prefixes /build/work/f43a1db9c1a0389ec7418001d2a110d9a562/google3/runfiles/google3/third_party/llvm/llvm-project/llvm/test/CodeGen/X86/powi-int32min.ll

@cmtice
Copy link
Contributor

@cmtice cmtice commented on 8f966ce May 3, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe you should revert this commit?

@RKSimon
Copy link
Collaborator

@RKSimon RKSimon commented on 8f966ce May 3, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed by 1c61acc?

Please sign in to comment.