This is an archive of the discontinued LLVM Phabricator instance.

Differential D131757

[instcombine] Test for zero initialisation optimisation of a product given fast flags
ClosedPublic

Authored by zjaffal on Aug 12 2022, 3:10 AM.

Details

Reviewers
fhahn
Commits
rG468a9d6d2a5b: [instcombine] Test for zero initialisation optimisation of a product given fast…
Summary

Given a zero initialised variable iterating through an array and multiplying its elements, the optimiser should be able to remove the loop safely and return the initial value because fast math flags are enabled.

double arr_d[1000];
double f_prod = 0;
for (size_t i = 0; i < 1000; i++){
  f_prod *= arr_d[i];
}

The loop here can be deleted and we can return f_prod instead

Diff Detail

Event Timeline

zjaffal created this revision.Aug 12 2022, 3:10 AM
Herald added a project: Restricted Project. · View Herald TranscriptAug 12 2022, 3:10 AM
zjaffal added a reviewer: fhahn.Aug 12 2022, 3:12 AM
zjaffal added a child revision: D131672: [instcombine] Optimise for zero initialisation of product given fast flags are enabled.Aug 12 2022, 3:37 AM
Harbormaster completed remote builds in B180876: Diff 452124.Aug 12 2022, 4:56 AM
zjaffal published this revision for review.Aug 12 2022, 5:54 AM
Herald added a project: Restricted Project. · View Herald TranscriptAug 12 2022, 5:54 AM
Herald added a subscriber: llvm-commits. · View Herald Transcript
fhahn added a comment.Aug 12 2022, 8:12 AM

I think the title/description needs updating to make clear this just adds tests.

[instcombine] Check for zero initialisation optimisation of a product given fast flags are enabled in clang.

Also it's not really Clang related, as this is a generic IR optimization, so it is best to drop the reference to Clang from the title.

llvm/test/Transforms/InstCombine/remove-loop-phi-fastmul.ll
22

Could you pass this as an argument to the tests to avoid reading memory that's not initialized?

167

I think it would be better to pass this value as argument.

242

I think it would be better to pass this condition as argument rather than having a trivial comparison.

zjaffal updated this revision to Diff 452599.Aug 15 2022, 1:03 AM

pass constants and arrays as function parameters

Harbormaster completed remote builds in B181228: Diff 452599.Aug 15 2022, 2:30 AM
zjaffal updated this revision to Diff 452705.Aug 15 2022, 9:29 AM
zjaffal marked 3 inline comments as done.

Updating D131757: [instcombine] Check for zero initialisation optimisation of a product given fast flags are enabled in clang.

Herald added a subscriber: hiraditya. · View Herald TranscriptAug 15 2022, 9:29 AM
Harbormaster completed remote builds in B181308: Diff 452705.Aug 15 2022, 9:30 AM
fhahn added a comment.Aug 15 2022, 9:32 AM

It looks like you accidentally uploaded the wrong patch here. Originally this just added the tests. (Also the title needs updating to avoid confusion)

zjaffal updated this revision to Diff 452712.Aug 15 2022, 9:35 AM

Updating D131757: [instcombine] Check for zero initialisation optimisation of a product given fast flags are enabled in clang.

Harbormaster completed remote builds in B181313: Diff 452712.Aug 15 2022, 9:36 AM
zjaffal updated this revision to Diff 452717.Aug 15 2022, 9:41 AM

Updating D131757: [instcombine] Check for zero initialisation optimisation of a product given fast flags are enabled in clang.

zjaffal retitled this revision from [instcombine] Check for zero initialisation optimisation of a product given fast flags are enabled in clang. to [instcombine] Test for zero initialisation optimisation of a product given fast flags.Aug 15 2022, 10:09 AM
Harbormaster completed remote builds in B181316: Diff 452717.Aug 15 2022, 11:24 AM
fhahn added inline comments.Aug 16 2022, 1:02 AM
llvm/test/Transforms/InstCombine/remove-loop-phi-fastmul.ll
234

might be good to also have a variant of this test that doesn't have 0.0 as incoming value.

zjaffal updated this revision to Diff 452933.Aug 16 2022, 2:51 AM

Updating D131757: [instcombine] Test for zero initialisation optimisation of a product given fast flags

Harbormaster completed remote builds in B181472: Diff 452933.Aug 16 2022, 3:35 AM
fhahn accepted this revision.Aug 16 2022, 5:54 AM

LGTM, thanks!

This revision is now accepted and ready to land.Aug 16 2022, 5:54 AM
Closed by commit rG468a9d6d2a5b: [instcombine] Test for zero initialisation optimisation of a product given fast… (authored by zjaffal, committed by fhahn). · Explain WhyAug 16 2022, 6:08 AM
This revision was automatically updated to reflect the committed changes.
fhahn added a commit: rG468a9d6d2a5b: [instcombine] Test for zero initialisation optimisation of a product given fast….

Revision Contents

PathSize
llvm/
test/
Transforms/
InstCombine/
283 lines

Diff 452979

llvm/test/Transforms/InstCombine/remove-loop-phi-fastmul.ll

  • This file was added.
; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
; RUN: opt < %s -passes=instcombine -S | FileCheck %s
define double @test_mul_fast_flags(ptr %arr_d) {
; CHECK-LABEL: @test_mul_fast_flags(
; CHECK-NEXT: entry:
; CHECK-NEXT: br label [[FOR_BODY:%.*]]
; CHECK: for.body:
; CHECK-NEXT: [[I_02:%.*]] = phi i64 [ 0, [[ENTRY:%.*]] ], [ [[INC:%.*]], [[FOR_BODY]] ]
; CHECK-NEXT: [[F_PROD_01:%.*]] = phi double [ 0.000000e+00, [[ENTRY]] ], [ [[MUL:%.*]], [[FOR_BODY]] ]
; CHECK-NEXT: [[ARRAYIDX:%.*]] = getelementptr inbounds [1000 x double], ptr [[ARR_D:%.*]], i64 0, i64 [[I_02]]
; CHECK-NEXT: [[TMP0:%.*]] = load double, ptr [[ARRAYIDX]], align 8
; CHECK-NEXT: [[MUL]] = fmul fast double [[F_PROD_01]], [[TMP0]]
; CHECK-NEXT: [[INC]] = add i64 [[I_02]], 1
; CHECK-NEXT: [[CMP:%.*]] = icmp ult i64 [[INC]], 1000
; CHECK-NEXT: br i1 [[CMP]], label [[FOR_BODY]], label [[END:%.*]]
; CHECK: end:
; CHECK-NEXT: ret double [[MUL]]
;
entry:
br label %for.body
for.body: ; preds = %entry, %for.body
fhahnUnsubmitted
Done
ReplyInline Actions

Could you pass this as an argument to the tests to avoid reading memory that's not initialized?

fhahn: Could you pass this as an argument to the tests to avoid reading memory that's not initialized?
%i.02 = phi i64 [ 0, %entry ], [ %inc, %for.body ]
%f_prod.01 = phi double [ 0.000000e+00, %entry ], [ %mul, %for.body ]
%arrayidx = getelementptr inbounds [1000 x double], ptr %arr_d, i64 0, i64 %i.02
%0 = load double, ptr %arrayidx, align 8
%mul = fmul fast double %f_prod.01, %0
%inc = add i64 %i.02, 1
%cmp = icmp ult i64 %inc, 1000
br i1 %cmp, label %for.body, label %end
end: ; preds = %for.body
%f_prod.0.lcssa = phi double [ %mul, %for.body ]
ret double %f_prod.0.lcssa
}
define double @test_nsz_nnan_flags_enabled(ptr %arr_d) {
; CHECK-LABEL: @test_nsz_nnan_flags_enabled(
; CHECK-NEXT: entry:
; CHECK-NEXT: br label [[FOR_BODY:%.*]]
; CHECK: for.body:
; CHECK-NEXT: [[I_02:%.*]] = phi i64 [ 0, [[ENTRY:%.*]] ], [ [[INC:%.*]], [[FOR_BODY]] ]
; CHECK-NEXT: [[F_PROD_01:%.*]] = phi double [ 0.000000e+00, [[ENTRY]] ], [ [[MUL:%.*]], [[FOR_BODY]] ]
; CHECK-NEXT: [[ARRAYIDX:%.*]] = getelementptr inbounds [1000 x double], ptr [[ARR_D:%.*]], i64 0, i64 [[I_02]]
; CHECK-NEXT: [[TMP0:%.*]] = load double, ptr [[ARRAYIDX]], align 8
; CHECK-NEXT: [[MUL]] = fmul nnan nsz double [[F_PROD_01]], [[TMP0]]
; CHECK-NEXT: [[INC]] = add i64 [[I_02]], 1
; CHECK-NEXT: [[CMP:%.*]] = icmp ult i64 [[INC]], 1000
; CHECK-NEXT: br i1 [[CMP]], label [[FOR_BODY]], label [[END:%.*]]
; CHECK: end:
; CHECK-NEXT: ret double [[MUL]]
;
entry:
br label %for.body
for.body: ; preds = %entry, %for.body
%i.02 = phi i64 [ 0, %entry ], [ %inc, %for.body ]
%f_prod.01 = phi double [ 0.000000e+00, %entry ], [ %mul, %for.body ]
%arrayidx = getelementptr inbounds [1000 x double], ptr %arr_d, i64 0, i64 %i.02
%0 = load double, ptr %arrayidx, align 8
%mul = fmul nsz nnan double %f_prod.01, %0
%inc = add i64 %i.02, 1
%cmp = icmp ult i64 %inc, 1000
br i1 %cmp, label %for.body, label %end
end: ; preds = %for.body
%f_prod.0.lcssa = phi double [ %mul, %for.body ]
ret double %f_prod.0.lcssa
}
define double @test_nnan_flag_enabled(ptr %arr_d) {
; CHECK-LABEL: @test_nnan_flag_enabled(
; CHECK-NEXT: entry:
; CHECK-NEXT: br label [[FOR_BODY:%.*]]
; CHECK: for.body:
; CHECK-NEXT: [[I_02:%.*]] = phi i64 [ 0, [[ENTRY:%.*]] ], [ [[INC:%.*]], [[FOR_BODY]] ]
; CHECK-NEXT: [[F_PROD_01:%.*]] = phi double [ 0.000000e+00, [[ENTRY]] ], [ [[MUL:%.*]], [[FOR_BODY]] ]
; CHECK-NEXT: [[ARRAYIDX:%.*]] = getelementptr inbounds [1000 x double], ptr [[ARR_D:%.*]], i64 0, i64 [[I_02]]
; CHECK-NEXT: [[TMP0:%.*]] = load double, ptr [[ARRAYIDX]], align 8
; CHECK-NEXT: [[MUL]] = fmul nnan double [[F_PROD_01]], [[TMP0]]
; CHECK-NEXT: [[INC]] = add i64 [[I_02]], 1
; CHECK-NEXT: [[CMP:%.*]] = icmp ult i64 [[INC]], 1000
; CHECK-NEXT: br i1 [[CMP]], label [[FOR_BODY]], label [[END:%.*]]
; CHECK: end:
; CHECK-NEXT: ret double [[MUL]]
;
entry:
br label %for.body
for.body: ; preds = %entry, %for.body
%i.02 = phi i64 [ 0, %entry ], [ %inc, %for.body ]
%f_prod.01 = phi double [ 0.000000e+00, %entry ], [ %mul, %for.body ]
%arrayidx = getelementptr inbounds [1000 x double], ptr %arr_d, i64 0, i64 %i.02
%0 = load double, ptr %arrayidx, align 8
%mul = fmul nnan double %f_prod.01, %0
%inc = add i64 %i.02, 1
%cmp = icmp ult i64 %inc, 1000
br i1 %cmp, label %for.body, label %end
end: ; preds = %for.body
%f_prod.0.lcssa = phi double [ %mul, %for.body ]
ret double %f_prod.0.lcssa
}
define double @test_ninf_flag_enabled(ptr %arr_d) {
; CHECK-LABEL: @test_ninf_flag_enabled(
; CHECK-NEXT: entry:
; CHECK-NEXT: br label [[FOR_BODY:%.*]]
; CHECK: for.body:
; CHECK-NEXT: [[I_02:%.*]] = phi i64 [ 0, [[ENTRY:%.*]] ], [ [[INC:%.*]], [[FOR_BODY]] ]
; CHECK-NEXT: [[F_PROD_01:%.*]] = phi double [ 0.000000e+00, [[ENTRY]] ], [ [[MUL:%.*]], [[FOR_BODY]] ]
; CHECK-NEXT: [[ARRAYIDX:%.*]] = getelementptr inbounds [1000 x double], ptr [[ARR_D:%.*]], i64 0, i64 [[I_02]]
; CHECK-NEXT: [[TMP0:%.*]] = load double, ptr [[ARRAYIDX]], align 8
; CHECK-NEXT: [[MUL]] = fmul ninf double [[F_PROD_01]], [[TMP0]]
; CHECK-NEXT: [[INC]] = add i64 [[I_02]], 1
; CHECK-NEXT: [[CMP:%.*]] = icmp ult i64 [[INC]], 1000
; CHECK-NEXT: br i1 [[CMP]], label [[FOR_BODY]], label [[END:%.*]]
; CHECK: end:
; CHECK-NEXT: ret double [[MUL]]
;
entry:
br label %for.body
for.body: ; preds = %entry, %for.body
%i.02 = phi i64 [ 0, %entry ], [ %inc, %for.body ]
%f_prod.01 = phi double [ 0.000000e+00, %entry ], [ %mul, %for.body ]
%arrayidx = getelementptr inbounds [1000 x double], ptr %arr_d, i64 0, i64 %i.02
%0 = load double, ptr %arrayidx, align 8
%mul = fmul ninf double %f_prod.01, %0
%inc = add i64 %i.02, 1
%cmp = icmp ult i64 %inc, 1000
br i1 %cmp, label %for.body, label %end
end: ; preds = %for.body
%f_prod.0.lcssa = phi double [ %mul, %for.body ]
ret double %f_prod.0.lcssa
}
define double @test_nsz_flag_enabled(ptr %arr_d) {
; CHECK-LABEL: @test_nsz_flag_enabled(
; CHECK-NEXT: entry:
; CHECK-NEXT: br label [[FOR_BODY:%.*]]
; CHECK: for.body:
; CHECK-NEXT: [[I_02:%.*]] = phi i64 [ 0, [[ENTRY:%.*]] ], [ [[INC:%.*]], [[FOR_BODY]] ]
; CHECK-NEXT: [[F_PROD_01:%.*]] = phi double [ 0.000000e+00, [[ENTRY]] ], [ [[MUL:%.*]], [[FOR_BODY]] ]
; CHECK-NEXT: [[ARRAYIDX:%.*]] = getelementptr inbounds [1000 x double], ptr [[ARR_D:%.*]], i64 0, i64 [[I_02]]
; CHECK-NEXT: [[TMP0:%.*]] = load double, ptr [[ARRAYIDX]], align 8
; CHECK-NEXT: [[MUL]] = fmul nsz double [[F_PROD_01]], [[TMP0]]
; CHECK-NEXT: [[INC]] = add i64 [[I_02]], 1
; CHECK-NEXT: [[CMP:%.*]] = icmp ult i64 [[INC]], 1000
; CHECK-NEXT: br i1 [[CMP]], label [[FOR_BODY]], label [[END:%.*]]
; CHECK: end:
; CHECK-NEXT: ret double [[MUL]]
;
entry:
br label %for.body
for.body: ; preds = %entry, %for.body
%i.02 = phi i64 [ 0, %entry ], [ %inc, %for.body ]
%f_prod.01 = phi double [ 0.000000e+00, %entry ], [ %mul, %for.body ]
%arrayidx = getelementptr inbounds [1000 x double], ptr %arr_d, i64 0, i64 %i.02
%0 = load double, ptr %arrayidx, align 8
%mul = fmul nsz double %f_prod.01, %0
%inc = add i64 %i.02, 1
%cmp = icmp ult i64 %inc, 1000
br i1 %cmp, label %for.body, label %end
fhahnUnsubmitted
Done
ReplyInline Actions

I think it would be better to pass this value as argument.

fhahn: I think it would be better to pass this value as argument.
end: ; preds = %for.body
%f_prod.0.lcssa = phi double [ %mul, %for.body ]
ret double %f_prod.0.lcssa
}
define double @test_phi_initalise_to_non_zero(ptr %arr_d) {
; CHECK-LABEL: @test_phi_initalise_to_non_zero(
; CHECK-NEXT: entry:
; CHECK-NEXT: br label [[FOR_BODY:%.*]]
; CHECK: for.body:
; CHECK-NEXT: [[I_02:%.*]] = phi i64 [ 0, [[ENTRY:%.*]] ], [ [[INC:%.*]], [[FOR_BODY]] ]
; CHECK-NEXT: [[F_PROD_01:%.*]] = phi double [ 1.000000e+00, [[ENTRY]] ], [ [[MUL:%.*]], [[FOR_BODY]] ]
; CHECK-NEXT: [[ARRAYIDX:%.*]] = getelementptr inbounds [1000 x double], ptr [[ARR_D:%.*]], i64 0, i64 [[I_02]]
; CHECK-NEXT: [[TMP0:%.*]] = load double, ptr [[ARRAYIDX]], align 8
; CHECK-NEXT: [[MUL]] = fmul fast double [[F_PROD_01]], [[TMP0]]
; CHECK-NEXT: [[INC]] = add i64 [[I_02]], 1
; CHECK-NEXT: [[CMP:%.*]] = icmp ult i64 [[INC]], 1000
; CHECK-NEXT: br i1 [[CMP]], label [[FOR_BODY]], label [[END:%.*]]
; CHECK: end:
; CHECK-NEXT: ret double [[MUL]]
;
entry:
br label %for.body
for.body: ; preds = %entry, %for.body
%i.02 = phi i64 [ 0, %entry ], [ %inc, %for.body ]
%f_prod.01 = phi double [ 1.000000e+00, %entry ], [ %mul, %for.body ]
%arrayidx = getelementptr inbounds [1000 x double], ptr %arr_d, i64 0, i64 %i.02
%0 = load double, ptr %arrayidx, align 8
%mul = fmul fast double %f_prod.01, %0
%inc = add i64 %i.02, 1
%cmp = icmp ult i64 %inc, 1000
br i1 %cmp, label %for.body, label %end
end: ; preds = %for.body
%f_prod.0.lcssa = phi double [ %mul, %for.body ]
ret double %f_prod.0.lcssa
}
define double @test_multiple_phi_operands(ptr %arr_d, i1 %entry_cond) {
; CHECK-LABEL: @test_multiple_phi_operands(
; CHECK-NEXT: entry:
; CHECK-NEXT: br i1 [[ENTRY_COND:%.*]], label [[FOR_BODY:%.*]], label [[ENTRY_2:%.*]]
; CHECK: entry_2:
; CHECK-NEXT: br label [[FOR_BODY]]
; CHECK: for.body:
; CHECK-NEXT: [[I_02:%.*]] = phi i64 [ 0, [[ENTRY:%.*]] ], [ 0, [[ENTRY_2]] ], [ [[INC:%.*]], [[FOR_BODY]] ]
; CHECK-NEXT: [[F_PROD_01:%.*]] = phi double [ 0.000000e+00, [[ENTRY]] ], [ 0.000000e+00, [[ENTRY_2]] ], [ [[MUL:%.*]], [[FOR_BODY]] ]
; CHECK-NEXT: [[ARRAYIDX:%.*]] = getelementptr inbounds [1000 x double], ptr [[ARR_D:%.*]], i64 0, i64 [[I_02]]
; CHECK-NEXT: [[TMP0:%.*]] = load double, ptr [[ARRAYIDX]], align 8
; CHECK-NEXT: [[MUL]] = fmul fast double [[F_PROD_01]], [[TMP0]]
; CHECK-NEXT: [[INC]] = add i64 [[I_02]], 1
; CHECK-NEXT: [[CMP:%.*]] = icmp ult i64 [[INC]], 1000
; CHECK-NEXT: br i1 [[CMP]], label [[FOR_BODY]], label [[END:%.*]]
; CHECK: end:
; CHECK-NEXT: ret double [[MUL]]
;
entry:
br i1 %entry_cond, label %for.body, label %entry_2
entry_2:
br label %for.body
for.body: ; preds = %entry, %for.body
%i.02 = phi i64 [ 0, %entry ], [0, %entry_2], [ %inc, %for.body ]
%f_prod.01 = phi double [ 0.0, %entry ], [0.0, %entry_2], [ %mul, %for.body ]
%arrayidx = getelementptr inbounds [1000 x double], ptr %arr_d, i64 0, i64 %i.02
fhahnUnsubmitted
Not Done
ReplyInline Actions

might be good to also have a variant of this test that doesn't have 0.0 as incoming value.

fhahn: might be good to also have a variant of this test that doesn't have 0.0 as incoming value.
%0 = load double, ptr %arrayidx, align 8
%mul = fmul fast double %f_prod.01, %0
%inc = add i64 %i.02, 1
%cmp = icmp ult i64 %inc, 1000
br i1 %cmp, label %for.body, label %end
end: ; preds = %for.body
%f_prod.0.lcssa = phi double [ %mul, %for.body ]
fhahnUnsubmitted
Done
ReplyInline Actions

I think it would be better to pass this condition as argument rather than having a trivial comparison.

fhahn: I think it would be better to pass this condition as argument rather than having a trivial…
ret double %f_prod.0.lcssa
}
define double @test_multiple_phi_operands_with_non_zero(ptr %arr_d, i1 %entry_cond) {
; CHECK-LABEL: @test_multiple_phi_operands_with_non_zero(
; CHECK-NEXT: entry:
; CHECK-NEXT: br i1 [[ENTRY_COND:%.*]], label [[FOR_BODY:%.*]], label [[ENTRY_2:%.*]]
; CHECK: entry_2:
; CHECK-NEXT: br label [[FOR_BODY]]
; CHECK: for.body:
; CHECK-NEXT: [[I_02:%.*]] = phi i64 [ 0, [[ENTRY:%.*]] ], [ 0, [[ENTRY_2]] ], [ [[INC:%.*]], [[FOR_BODY]] ]
; CHECK-NEXT: [[F_PROD_01:%.*]] = phi double [ 1.000000e+00, [[ENTRY]] ], [ 0.000000e+00, [[ENTRY_2]] ], [ [[MUL:%.*]], [[FOR_BODY]] ]
; CHECK-NEXT: [[ARRAYIDX:%.*]] = getelementptr inbounds [1000 x double], ptr [[ARR_D:%.*]], i64 0, i64 [[I_02]]
; CHECK-NEXT: [[TMP0:%.*]] = load double, ptr [[ARRAYIDX]], align 8
; CHECK-NEXT: [[MUL]] = fmul fast double [[F_PROD_01]], [[TMP0]]
; CHECK-NEXT: [[INC]] = add i64 [[I_02]], 1
; CHECK-NEXT: [[CMP:%.*]] = icmp ult i64 [[INC]], 1000
; CHECK-NEXT: br i1 [[CMP]], label [[FOR_BODY]], label [[END:%.*]]
; CHECK: end:
; CHECK-NEXT: ret double [[MUL]]
;
entry:
br i1 %entry_cond, label %for.body, label %entry_2
entry_2:
br label %for.body
for.body: ; preds = %entry, %for.body
%i.02 = phi i64 [ 0, %entry ], [0, %entry_2], [ %inc, %for.body ]
%f_prod.01 = phi double [ 1.0, %entry ], [0.0, %entry_2], [ %mul, %for.body ]
%arrayidx = getelementptr inbounds [1000 x double], ptr %arr_d, i64 0, i64 %i.02
%0 = load double, ptr %arrayidx, align 8
%mul = fmul fast double %f_prod.01, %0
%inc = add i64 %i.02, 1
%cmp = icmp ult i64 %inc, 1000
br i1 %cmp, label %for.body, label %end
end: ; preds = %for.body
%f_prod.0.lcssa = phi double [ %mul, %for.body ]
ret double %f_prod.0.lcssa
}