Skip to content

Commit

Permalink
[SimplifyLibCalls] Return Value from optimizeSinCosPi when making change
Browse files Browse the repository at this point in the history 
Or else InstCombine can incorrectly report that no change has been made.

This optimization doesn't really fit into InstCombine since it optimizes multiple instructions at once; there's likely a more comprehensive fix.

Reviewed By: nikic

Differential Revision: https://reviews.llvm.org/D146064
  • Loading branch information
@aeubanks
aeubanks committed Mar 14, 2023
1 parent 2ef4162 commit 093b264
Show file tree
Hide file tree
Showing 3 changed files with 7 additions and 16 deletions.
2 changes: 1 addition & 1 deletion llvm/include/llvm/Transforms/Utils/SimplifyLibCalls.h
View file
Expand Up @@ -198,7 +198,7 @@ class LibCallSimplifier {
Value *optimizeFMinFMax(CallInst *CI, IRBuilderBase &B);
Value *optimizeLog(CallInst *CI, IRBuilderBase &B);
Value *optimizeSqrt(CallInst *CI, IRBuilderBase &B);
Value *optimizeSinCosPi(CallInst *CI, IRBuilderBase &B);
Value *optimizeSinCosPi(CallInst *CI, bool IsSin, IRBuilderBase &B);
Value *optimizeTan(CallInst *CI, IRBuilderBase &B);
// Wrapper for all floating point library call optimizations
Value *optimizeFloatingPointLibCall(CallInst *CI, LibFunc Func,
Expand Down
7 changes: 4 additions & 3 deletions llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp
View file
Expand Up @@ -2588,7 +2588,7 @@ static bool insertSinCosCall(IRBuilderBase &B, Function *OrigCallee, Value *Arg,
return true;
}

Value *LibCallSimplifier::optimizeSinCosPi(CallInst *CI, IRBuilderBase &B) {
Value *LibCallSimplifier::optimizeSinCosPi(CallInst *CI, bool IsSin, IRBuilderBase &B) {
// Make sure the prototype is as expected, otherwise the rest of the
// function is probably invalid and likely to abort.
if (!isTrigLibCall(CI))
Expand Down Expand Up @@ -2627,7 +2627,7 @@ Value *LibCallSimplifier::optimizeSinCosPi(CallInst *CI, IRBuilderBase &B) {
replaceTrigInsts(CosCalls, Cos);
replaceTrigInsts(SinCosCalls, SinCos);

return nullptr;
return IsSin ? Sin : Cos;
}

void LibCallSimplifier::classifyArgUse(
Expand Down Expand Up @@ -3470,9 +3470,10 @@ Value *LibCallSimplifier::optimizeFloatingPointLibCall(CallInst *CI,
switch (Func) {
case LibFunc_sinpif:
case LibFunc_sinpi:
return optimizeSinCosPi(CI, /*IsSin*/true, Builder);
case LibFunc_cospif:
case LibFunc_cospi:
return optimizeSinCosPi(CI, Builder);
return optimizeSinCosPi(CI, /*IsSin*/false, Builder);
case LibFunc_powf:
case LibFunc_pow:
case LibFunc_powl:
Expand Down
14 changes: 2 additions & 12 deletions llvm/test/Transforms/InstCombine/sincospi.ll
View file
Expand Up @@ -24,8 +24,7 @@ define float @test_instbased_f32() {
; CHECK-FLOAT-IN-VEC-NEXT: [[SINCOSPI:%.*]] = call <2 x float> @__sincospif_stret(float [[VAL]])
; CHECK-FLOAT-IN-VEC-NEXT: [[SINPI:%.*]] = extractelement <2 x float> [[SINCOSPI]], i64 0
; CHECK-FLOAT-IN-VEC-NEXT: [[COSPI:%.*]] = extractelement <2 x float> [[SINCOSPI]], i64 1
; CHECK-FLOAT-IN-VEC-NEXT: [[SIN:%.*]] = call float @__sinpif(float [[VAL]]) #[[ATTR0:[0-9]+]]
; CHECK-FLOAT-IN-VEC-NEXT: [[COS:%.*]] = call float @__cospif(float [[VAL]]) #[[ATTR0]]
; CHECK-FLOAT-IN-VEC-NEXT: [[COS:%.*]] = call float @__cospif(float [[VAL]]) #[[ATTR0:[0-9]+]]
; CHECK-FLOAT-IN-VEC-NEXT: [[RES:%.*]] = fadd float [[SINPI]], [[COSPI]]
; CHECK-FLOAT-IN-VEC-NEXT: ret float [[RES]]
;
Expand All @@ -34,8 +33,7 @@ define float @test_instbased_f32() {
; CHECK-NEXT: [[SINCOSPI:%.*]] = call { float, float } @__sincospif_stret(float [[VAL]])
; CHECK-NEXT: [[SINPI:%.*]] = extractvalue { float, float } [[SINCOSPI]], 0
; CHECK-NEXT: [[COSPI:%.*]] = extractvalue { float, float } [[SINCOSPI]], 1
; CHECK-NEXT: [[SIN:%.*]] = call float @__sinpif(float [[VAL]]) #[[ATTR0:[0-9]+]]
; CHECK-NEXT: [[COS:%.*]] = call float @__cospif(float [[VAL]]) #[[ATTR0]]
; CHECK-NEXT: [[COS:%.*]] = call float @__cospif(float [[VAL]]) #[[ATTR0:[0-9]+]]
; CHECK-NEXT: [[RES:%.*]] = fadd float [[SINPI]], [[COSPI]]
; CHECK-NEXT: ret float [[RES]]
;
Expand All @@ -60,7 +58,6 @@ define float @test_instbased_f32_other_user(ptr %ptr) {
; CHECK-FLOAT-IN-VEC-NEXT: [[SINPI:%.*]] = extractelement <2 x float> [[SINCOSPI]], i64 0
; CHECK-FLOAT-IN-VEC-NEXT: [[COSPI:%.*]] = extractelement <2 x float> [[SINCOSPI]], i64 1
; CHECK-FLOAT-IN-VEC-NEXT: store float [[VAL]], ptr [[PTR:%.*]], align 4
; CHECK-FLOAT-IN-VEC-NEXT: [[SIN:%.*]] = call float @__sinpif(float [[VAL]]) #[[ATTR0]]
; CHECK-FLOAT-IN-VEC-NEXT: [[COS:%.*]] = call float @__cospif(float [[VAL]]) #[[ATTR0]]
; CHECK-FLOAT-IN-VEC-NEXT: [[RES:%.*]] = fadd float [[SINPI]], [[COSPI]]
; CHECK-FLOAT-IN-VEC-NEXT: ret float [[RES]]
Expand All @@ -71,7 +68,6 @@ define float @test_instbased_f32_other_user(ptr %ptr) {
; CHECK-NEXT: [[SINPI:%.*]] = extractvalue { float, float } [[SINCOSPI]], 0
; CHECK-NEXT: [[COSPI:%.*]] = extractvalue { float, float } [[SINCOSPI]], 1
; CHECK-NEXT: store float [[VAL]], ptr [[PTR:%.*]], align 4
; CHECK-NEXT: [[SIN:%.*]] = call float @__sinpif(float [[VAL]]) #[[ATTR0]]
; CHECK-NEXT: [[COS:%.*]] = call float @__cospif(float [[VAL]]) #[[ATTR0]]
; CHECK-NEXT: [[RES:%.*]] = fadd float [[SINPI]], [[COSPI]]
; CHECK-NEXT: ret float [[RES]]
Expand All @@ -97,7 +93,6 @@ define float @test_constant_f32() {
; CHECK-FLOAT-IN-VEC-NEXT: [[SINCOSPI:%.*]] = call <2 x float> @__sincospif_stret(float 1.000000e+00)
; CHECK-FLOAT-IN-VEC-NEXT: [[SINPI:%.*]] = extractelement <2 x float> [[SINCOSPI]], i64 0
; CHECK-FLOAT-IN-VEC-NEXT: [[COSPI:%.*]] = extractelement <2 x float> [[SINCOSPI]], i64 1
; CHECK-FLOAT-IN-VEC-NEXT: [[SIN:%.*]] = call float @__sinpif(float 1.000000e+00) #[[ATTR0]]
; CHECK-FLOAT-IN-VEC-NEXT: [[COS:%.*]] = call float @__cospif(float 1.000000e+00) #[[ATTR0]]
; CHECK-FLOAT-IN-VEC-NEXT: [[RES:%.*]] = fadd float [[SINPI]], [[COSPI]]
; CHECK-FLOAT-IN-VEC-NEXT: ret float [[RES]]
Expand All @@ -106,7 +101,6 @@ define float @test_constant_f32() {
; CHECK-NEXT: [[SINCOSPI:%.*]] = call { float, float } @__sincospif_stret(float 1.000000e+00)
; CHECK-NEXT: [[SINPI:%.*]] = extractvalue { float, float } [[SINCOSPI]], 0
; CHECK-NEXT: [[COSPI:%.*]] = extractvalue { float, float } [[SINCOSPI]], 1
; CHECK-NEXT: [[SIN:%.*]] = call float @__sinpif(float 1.000000e+00) #[[ATTR0]]
; CHECK-NEXT: [[COS:%.*]] = call float @__cospif(float 1.000000e+00) #[[ATTR0]]
; CHECK-NEXT: [[RES:%.*]] = fadd float [[SINPI]], [[COSPI]]
; CHECK-NEXT: ret float [[RES]]
Expand All @@ -131,7 +125,6 @@ define double @test_instbased_f64() {
; CHECK-FLOAT-IN-VEC-NEXT: [[SINCOSPI:%.*]] = call { double, double } @__sincospi_stret(double [[VAL]])
; CHECK-FLOAT-IN-VEC-NEXT: [[SINPI:%.*]] = extractvalue { double, double } [[SINCOSPI]], 0
; CHECK-FLOAT-IN-VEC-NEXT: [[COSPI:%.*]] = extractvalue { double, double } [[SINCOSPI]], 1
; CHECK-FLOAT-IN-VEC-NEXT: [[SIN:%.*]] = call double @__sinpi(double [[VAL]]) #[[ATTR0]]
; CHECK-FLOAT-IN-VEC-NEXT: [[COS:%.*]] = call double @__cospi(double [[VAL]]) #[[ATTR0]]
; CHECK-FLOAT-IN-VEC-NEXT: [[RES:%.*]] = fadd double [[SINPI]], [[COSPI]]
; CHECK-FLOAT-IN-VEC-NEXT: ret double [[RES]]
Expand All @@ -141,7 +134,6 @@ define double @test_instbased_f64() {
; CHECK-NEXT: [[SINCOSPI:%.*]] = call { double, double } @__sincospi_stret(double [[VAL]])
; CHECK-NEXT: [[SINPI:%.*]] = extractvalue { double, double } [[SINCOSPI]], 0
; CHECK-NEXT: [[COSPI:%.*]] = extractvalue { double, double } [[SINCOSPI]], 1
; CHECK-NEXT: [[SIN:%.*]] = call double @__sinpi(double [[VAL]]) #[[ATTR0]]
; CHECK-NEXT: [[COS:%.*]] = call double @__cospi(double [[VAL]]) #[[ATTR0]]
; CHECK-NEXT: [[RES:%.*]] = fadd double [[SINPI]], [[COSPI]]
; CHECK-NEXT: ret double [[RES]]
Expand All @@ -167,7 +159,6 @@ define double @test_constant_f64() {
; CHECK-FLOAT-IN-VEC-NEXT: [[SINCOSPI:%.*]] = call { double, double } @__sincospi_stret(double 1.000000e+00)
; CHECK-FLOAT-IN-VEC-NEXT: [[SINPI:%.*]] = extractvalue { double, double } [[SINCOSPI]], 0
; CHECK-FLOAT-IN-VEC-NEXT: [[COSPI:%.*]] = extractvalue { double, double } [[SINCOSPI]], 1
; CHECK-FLOAT-IN-VEC-NEXT: [[SIN:%.*]] = call double @__sinpi(double 1.000000e+00) #[[ATTR0]]
; CHECK-FLOAT-IN-VEC-NEXT: [[COS:%.*]] = call double @__cospi(double 1.000000e+00) #[[ATTR0]]
; CHECK-FLOAT-IN-VEC-NEXT: [[RES:%.*]] = fadd double [[SINPI]], [[COSPI]]
; CHECK-FLOAT-IN-VEC-NEXT: ret double [[RES]]
Expand All @@ -176,7 +167,6 @@ define double @test_constant_f64() {
; CHECK-NEXT: [[SINCOSPI:%.*]] = call { double, double } @__sincospi_stret(double 1.000000e+00)
; CHECK-NEXT: [[SINPI:%.*]] = extractvalue { double, double } [[SINCOSPI]], 0
; CHECK-NEXT: [[COSPI:%.*]] = extractvalue { double, double } [[SINCOSPI]], 1
; CHECK-NEXT: [[SIN:%.*]] = call double @__sinpi(double 1.000000e+00) #[[ATTR0]]
; CHECK-NEXT: [[COS:%.*]] = call double @__cospi(double 1.000000e+00) #[[ATTR0]]
; CHECK-NEXT: [[RES:%.*]] = fadd double [[SINPI]], [[COSPI]]
; CHECK-NEXT: ret double [[RES]]
Expand Down

0 comments on commit 093b264

Please sign in to comment.