[PATCH] D75799: [JumpThreading] Don't create PHI nodes with "is.constant" values
Bill Wendling via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Sun Mar 8 18:38:28 PDT 2020
void added a comment.
In D75799#1911594 <https://reviews.llvm.org/D75799#1911594>, @nikic wrote:
> @void From your example, I still don't really get what the problem is / why this has to be done in JumpThreading in particular. The `%6` phi that gets inserted here looks harmless to me -- the phi is trivial and will get eliminated by any simplification pass later in the pipeline (though I'm not entirely sure why a single-arg phi gets created in the first place).
>
> What you seem to be really doing here is to move the is.constant lowering from the LowerConstantIntrinsics pass into the JumpThreading pass. It sounds to me like some kind of phase ordering issue where LowerConstantIntrinsics runs too late for the necessary DCE to be performed. Is that right? What does the IR look like at the point where the lowering is (currently) performed?
[@lebedev.ri this is in response to your question as well.]
Here's the relevant IR right before lower constant intrinsics (in the code snippets, look for calls to `__bad_copy_*`, those are the ones we want to get rid of):
if.then607: ; preds = %if.end603
%176 = call i32 asm sideeffect "call __put_user_4", "={ax},0,{cx},~{ebx},~{dirflag},~{fpsr},~{flags}"(i32 24, i32* %optlen) #9, !dbg !24792, !srcloc !24793
%tobool613 = icmp eq i32 %176, 0, !dbg !24794
br i1 %tobool613, label %if.then.i188, label %cleanup644, !dbg !24795, !prof !15173, !misexpect !14134
if.end616: ; preds = %if.end603
%sext220 = shl i64 %asmresult590, 32, !dbg !24796
%conv617 = ashr exact i64 %sext220, 32, !dbg !24796
%cmp3.i.i181 = icmp ugt i32 %conv592, 24, !dbg !24802
%177 = call i1 @llvm.is.constant.i64(i64 %conv617) #9, !dbg !24800
br i1 %cmp3.i.i181, label %if.then.i.i182, label %if.end12.i.i185, !dbg !24803, !prof !14133, !misexpect !14134
if.then.i.i182: ; preds = %if.end616
br i1 %177, label %if.else.i.i184, label %if.then7.i.i183, !dbg !24804, !prof !15547
if.then7.i.i183: ; preds = %if.then.i.i182
call void (i8*, ...) @__warn_printk(i8* getelementptr inbounds ([38 x i8], [38 x i8]* @.str.16, i64 0, i64 0), i32 24, i64 %conv617) #11, !dbg !24808
call void asm sideeffect "...", "i,i,i,i,~{dirflag},~{fpsr},~{flags}"(i8* getelementptr inbounds ([30 x i8], [30 x i8]* @.str.17, i64 0, i64 0), i32 127, i32 2305, i64 12) #9, !dbg !24809, !srcloc !24342
br label %copy_from_user.exit, !dbg !24810
if.else.i.i184: ; preds = %if.then.i.i182
call void @__bad_copy_to() #11, !dbg !24811
br label %copy_from_user.exit
if.end12.i.i185: ; preds = %if.end616
br i1 %177, label %if.then.i188, label %if.then.i.i.i186, !dbg !24814
if.then.i.i.i186: ; preds = %if.end12.i.i185
call void @__check_object_size(i8* nonnull %172, i64 %conv617, i1 zeroext false) #11, !dbg !24815
br label %if.then.i188, !dbg !24815
if.then.i188: ; preds = %if.then607, %if.then.i.i.i186, %if.end12.i.i185
%len.0208219 = phi i32 [ %conv592, %if.then.i.i.i186 ], [ %conv592, %if.end12.i.i185 ], [ 24, %if.then607 ]
%conv617210218 = phi i64 [ %conv617, %if.then.i.i.i186 ], [ %conv617, %if.end12.i.i185 ], [ 24, %if.then607 ]
%178 = phi i1 [ false, %if.then.i.i.i186 ], [ true, %if.end12.i.i185 ], [ true, %if.then607 ]
%call2.i187 = call i64 @_copy_from_user(i8* nonnull %172, i8* %optval, i64 %conv617210218) #11, !dbg !24816
br label %copy_from_user.exit, !dbg !24817
copy_from_user.exit: ; preds = %if.then7.i.i183, %if.else.i.i184, %if.then.i188
%179 = phi i1 [ %178, %if.then.i188 ], [ false, %if.then7.i.i183 ], [ true, %if.else.i.i184 ]
%cmp3.i.i181212 = phi i1 [ false, %if.then.i188 ], [ true, %if.then7.i.i183 ], [ true, %if.else.i.i184 ]
%conv617209 = phi i64 [ %conv617210218, %if.then.i188 ], [ %conv617, %if.then7.i.i183 ], [ %conv617, %if.else.i.i184 ]
%len.0207 = phi i32 [ %len.0208219, %if.then.i188 ], [ %conv592, %if.then7.i.i183 ], [ %conv592, %if.else.i.i184 ]
%n.addr.0.i189 = phi i64 [ %call2.i187, %if.then.i188 ], [ %conv617, %if.then7.i.i183 ], [ %conv617, %if.else.i.i184 ]
%tobool619 = icmp eq i64 %n.addr.0.i189, 0, !dbg !24818
br i1 %tobool619, label %if.end621, label %cleanup644, !dbg !24819
if.end621: ; preds = %copy_from_user.exit
call void @lock_sock_nested(%struct.sock* %sk, i32 0) #11, !dbg !24822
%call622 = call fastcc i32 @tcp_zerocopy_receive(%struct.sock* %sk, %struct.tcp_zerocopy_receive* nonnull %zc) #10, !dbg !24823
%sk_err.i = getelementptr inbounds %struct.sock, %struct.sock* %sk, i64 0, i32 51, !dbg !24833
%180 = load i32, i32* %sk_err.i, align 8, !dbg !24833
%tobool.i190 = icmp eq i32 %180, 0, !dbg !24833
br i1 %tobool.i190, label %sock_error.exit, label %if.end.i, !dbg !24835, !prof !15173, !misexpect !15389
if.end.i: ; preds = %if.then632
%181 = call i32 asm sideeffect "xchgl $0, $1\0A", "=r,=*m,0,*m,~{memory},~{cc},~{dirflag},~{fpsr},~{flags}"(i32* %sk_err.i, i32 0, i32* %sk_err.i) #9, !dbg !24837, !srcloc !15261
%sub.i = sub i32 0, %181, !dbg !24838
br label %sock_error.exit, !dbg !24839
sock_error.exit: ; preds = %if.then632, %if.end.i
%retval.0.i = phi i32 [ %sub.i, %if.end.i ], [ 0, %if.then632 ], !dbg !24831
%err634 = getelementptr inbounds %struct.tcp_zerocopy_receive, %struct.tcp_zerocopy_receive* %zc, i64 0, i32 4, !dbg !24840
store i32 %retval.0.i, i32* %err634, align 4, !dbg !24841
br label %zerocopy_rcv_inq, !dbg !24842
zerocopy_rcv_inq: ; preds = %if.end621, %zerocopy_rcv_sk_err, %sock_error.exit
%182 = getelementptr inbounds %struct.sock, %struct.sock* %sk, i64 2, i32 0, i32 12, i32 0, i32 0, i64 3, !dbg !24846
%183 = load volatile i32, i32* %182, align 4, !dbg !24849
%184 = getelementptr inbounds %struct.sock, %struct.sock* %sk, i64 2, i32 0, i32 12, i32 0, i32 0, i64 2, !dbg !24851
%185 = load volatile i32, i32* %184, align 4, !dbg !24854
%sub.i191 = sub i32 %185, %183, !dbg !24856
%cmp.i192 = icmp slt i32 %sub.i191, 0, !dbg !24857
br i1 %cmp.i192, label %if.then.i193, label %lor.rhs.i, !dbg !24857, !prof !14133
lor.rhs.i: ; preds = %zerocopy_rcv_inq
%186 = load volatile i32, i32* %182, align 4, !dbg !24860
%cmp20.i = icmp eq i32 %183, %186, !dbg !24857
br i1 %cmp20.i, label %if.end.i194, label %if.then.i193, !dbg !24862, !prof !15173, !misexpect !14134
if.then.i193: ; preds = %lor.rhs.i, %zerocopy_rcv_inq
call void @lock_sock_nested(%struct.sock* %sk, i32 0) #11, !dbg !24865
%187 = load i32, i32* %184, align 8, !dbg !24866
%188 = load i32, i32* %182, align 4, !dbg !24867
%sub24.i = sub i32 %187, %188, !dbg !24868
call void @release_sock(%struct.sock* %sk) #11, !dbg !24869
br label %if.end.i194, !dbg !24870
if.end.i194: ; preds = %if.then.i193, %lor.rhs.i
%inq.0.i = phi i32 [ %sub24.i, %if.then.i193 ], [ %sub.i191, %lor.rhs.i ], !dbg !24844
%cmp25.i = icmp eq i32 %inq.0.i, 0, !dbg !24871
br i1 %cmp25.i, label %land.lhs.true.i, label %tcp_inq_hint.exit, !dbg !24872
land.lhs.true.i: ; preds = %if.end.i194
%skc_flags.i.i = getelementptr inbounds %struct.sock, %struct.sock* %sk, i64 0, i32 0, i32 15, i32 0, !dbg !24875
%189 = load volatile i64, i64* %skc_flags.i.i, align 8, !dbg !24878
%and1.i.i.i = lshr i64 %189, 1, !dbg !24879
%190 = trunc i64 %and1.i.i.i to i32, !dbg !24879
%191 = and i32 %190, 1, !dbg !24879
br label %tcp_inq_hint.exit, !dbg !24879
tcp_inq_hint.exit: ; preds = %if.end.i194, %land.lhs.true.i
%call636195 = phi i32 [ %191, %land.lhs.true.i ], [ %inq.0.i, %if.end.i194 ]
%inq = getelementptr inbounds %struct.tcp_zerocopy_receive, %struct.tcp_zerocopy_receive* %zc, i64 0, i32 3, !dbg !24880
store i32 %call636195, i32* %inq, align 8, !dbg !24881
br label %zerocopy_rcv_out, !dbg !24882
zerocopy_rcv_out: ; preds = %if.end621, %tcp_inq_hint.exit
call void @llvm.dbg.label(metadata !24079), !dbg !24883
%tobool637 = icmp eq i32 %call622, 0, !dbg !24884
br i1 %tobool637, label %land.lhs.true638, label %cleanup644, !dbg !24886
land.lhs.true638: ; preds = %zerocopy_rcv_out
br i1 %cmp3.i.i181212, label %if.then.i.i39, label %if.end12.i.i42, !dbg !24891, !prof !14133, !misexpect !14134
if.then.i.i39: ; preds = %land.lhs.true638
br i1 %179, label %if.else.i.i41, label %if.then7.i.i40, !dbg !24892, !prof !15547
if.then7.i.i40: ; preds = %if.then.i.i39
call void (i8*, ...) @__warn_printk(i8* getelementptr inbounds ([38 x i8], [38 x i8]* @.str.16, i64 0, i64 0), i32 24, i64 %conv617209) #11, !dbg !24896
call void asm sideeffect "...", "i,i,i,i,~{dirflag},~{fpsr},~{flags}"(i8* getelementptr inbounds ([30 x i8], [30 x i8]* @.str.17, i64 0, i64 0), i32 127, i32 2305, i64 12) #9, !dbg !24897, !srcloc !24342
br label %copy_to_user.exit47, !dbg !24898
if.else.i.i41: ; preds = %if.then.i.i39
call void @__bad_copy_from() #11, !dbg !24899
br label %copy_to_user.exit47, !dbg !24899
The code paths in question are:
- %177 (llvm.is.constant) determines if __bad_copy_to() is executed. It should resolve to "false", so __bad_copy_to() is DCE'd.
- %179 (PHI node) determines if __bad_copy_from() is executed.
- %179 is a PHI of %178 (from %if.then.i188), false (from %if.then7.i.i183), and true (from %if.else.i.i184)
- %178 is also a PHI of false (from %if.then.i.i.i186), true (from %if.end12.i.i185), and true (from %if.then607)
Because %179 doesn't resolve to `false`, it keeps it around. So the question is "why are some of these values `true`?"
This is after jump threading:
if.end616: ; preds = %if.end603
%len.0 = phi i32 [ %conv592, %if.end603 ], !dbg !25087
%conv617 = sext i32 %len.0 to i64, !dbg !25088
%cmp3.i.i181 = icmp ugt i32 %len.0, 24, !dbg !25094
%177 = call i1 @llvm.is.constant.i64(i64 %conv617) #14, !dbg !25092
br i1 %cmp3.i.i181, label %if.then.i.i182, label %if.end12.i.i185, !dbg !25095, !prof !14149, !misexpect !14150
if.then.i.i182: ; preds = %if.end616
br i1 %177, label %if.else.i.i184, label %if.then7.i.i183, !dbg !25096, !prof !15638
if.then7.i.i183: ; preds = %if.then.i.i182
call void (i8*, ...) @__warn_printk(i8* getelementptr inbounds ([38 x i8], [38 x i8]* @.str.16, i64 0, i64 0), i32 24, i64 %conv617) #16, !dbg !25100
call void asm sideeffect "...", "i,i,i,i,~{dirflag},~{fpsr},~{flags}"(i8* getelementptr inbounds ([30 x i8], [30 x i8]* @.str.17, i64 0, i64 0), i32 127, i32 2305, i64 12) #14, !dbg !25101, !srcloc !24631
br label %copy_from_user.exit, !dbg !25102
if.else.i.i184: ; preds = %if.then.i.i182
call void @__bad_copy_to() #16, !dbg !25103
br label %copy_from_user.exit
if.end12.i.i185: ; preds = %if.end616
%178 = phi i1 [ %177, %if.end616 ]
%cmp3.i.i181213 = phi i1 [ %cmp3.i.i181, %if.end616 ]
%conv617210 = phi i64 [ %conv617, %if.end616 ]
%len.0208 = phi i32 [ %len.0, %if.end616 ]
br i1 %178, label %if.then.i188, label %if.then.i.i.i186, !dbg !25106
if.then.i.i.i186: ; preds = %if.end12.i.i185
call void @__check_object_size(i8* nonnull %172, i64 %conv617210, i1 zeroext false) #16, !dbg !25107
br label %if.then.i188, !dbg !25107
if.then.i188: ; preds = %if.then607, %if.then.i.i.i186, %if.end12.i.i185
%len.0208219 = phi i32 [ %len.0208, %if.then.i.i.i186 ], [ %len.0208, %if.end12.i.i185 ], [ 24, %if.then607 ]
%conv617210218 = phi i64 [ %conv617210, %if.then.i.i.i186 ], [ %conv617210, %if.end12.i.i185 ], [ 24, %if.then607 ]
%cmp3.i.i181213217 = phi i1 [ %cmp3.i.i181213, %if.then.i.i.i186 ], [ %cmp3.i.i181213, %if.end12.i.i185 ], [ false, %if.then607 ]
%179 = phi i1 [ %178, %if.then.i.i.i186 ], [ %178, %if.end12.i.i185 ], [ true, %if.then607 ]
%call2.i187 = call i64 @_copy_from_user(i8* nonnull %172, i8* %optval, i64 %conv617210218) #16, !dbg !25108
call void @llvm.dbg.value(metadata i64 %call2.i187, metadata !22165, metadata !DIExpression()) #14, !dbg !25090
br label %copy_from_user.exit, !dbg !25109
copy_from_user.exit: ; preds = %if.then7.i.i183, %if.else.i.i184, %if.then.i188
%180 = phi i1 [ %179, %if.then.i188 ], [ %177, %if.then7.i.i183 ], [ %177, %if.else.i.i184 ]
%cmp3.i.i181212 = phi i1 [ %cmp3.i.i181213217, %if.then.i188 ], [ %cmp3.i.i181, %if.then7.i.i183 ], [ %cmp3.i.i181, %if.else.i.i184 ]
%conv617209 = phi i64 [ %conv617210218, %if.then.i188 ], [ %conv617, %if.then7.i.i183 ], [ %conv617, %if.else.i.i184 ]
%len.0207 = phi i32 [ %len.0208219, %if.then.i188 ], [ %len.0, %if.then7.i.i183 ], [ %len.0, %if.else.i.i184 ]
%n.addr.0.i189 = phi i64 [ %call2.i187, %if.then.i188 ], [ %conv617, %if.then7.i.i183 ], [ %conv617, %if.else.i.i184 ]
call void @llvm.dbg.value(metadata i64 %n.addr.0.i189, metadata !22165, metadata !DIExpression()) #14, !dbg !25090
%tobool619 = icmp eq i64 %n.addr.0.i189, 0, !dbg !25110
br i1 %tobool619, label %if.end621, label %cleanup644, !dbg !25111
if.end621: ; preds = %copy_from_user.exit
call void @lock_sock_nested(%struct.sock* %sk, i32 0) #16, !dbg !25114
%call622 = call fastcc i32 @tcp_zerocopy_receive(%struct.sock* %sk, %struct.tcp_zerocopy_receive* nonnull %zc) #15, !dbg !25115
call void @release_sock(%struct.sock* %sk) #16, !dbg !25116
switch i32 %len.0207, label %zerocopy_rcv_out [
i32 24, label %zerocopy_rcv_sk_err
i32 20, label %zerocopy_rcv_inq
], !dbg !25117
zerocopy_rcv_sk_err: ; preds = %if.end621
call void @llvm.dbg.label(metadata !24366), !dbg !25118
%tobool631 = icmp eq i32 %call622, 0, !dbg !25119
br i1 %tobool631, label %if.then632, label %zerocopy_rcv_inq, !dbg !25121
if.then632: ; preds = %zerocopy_rcv_sk_err
%sk_err.i = getelementptr inbounds %struct.sock, %struct.sock* %sk, i64 0, i32 51, !dbg !25124
%181 = load i32, i32* %sk_err.i, align 8, !dbg !25124
%tobool.i190 = icmp eq i32 %181, 0, !dbg !25124
br i1 %tobool.i190, label %sock_error.exit, label %if.end.i, !dbg !25126, !prof !15210, !misexpect !15480
if.end.i: ; preds = %if.then632
%182 = call i32 asm sideeffect "xchgl $0, $1\0A", "=r,=*m,0,*m,~{memory},~{cc},~{dirflag},~{fpsr},~{flags}"(i32* %sk_err.i, i32 0, i32* %sk_err.i) #14, !dbg !25128, !srcloc !15298
%sub.i = sub i32 0, %182, !dbg !25129
br label %sock_error.exit, !dbg !25130
sock_error.exit: ; preds = %if.then632, %if.end.i
%retval.0.i = phi i32 [ %sub.i, %if.end.i ], [ 0, %if.then632 ], !dbg !25122
%err634 = getelementptr inbounds %struct.tcp_zerocopy_receive, %struct.tcp_zerocopy_receive* %zc, i64 0, i32 4, !dbg !25131
store i32 %retval.0.i, i32* %err634, align 4, !dbg !25132
br label %zerocopy_rcv_inq, !dbg !25133
zerocopy_rcv_inq: ; preds = %if.end621, %zerocopy_rcv_sk_err, %sock_error.exit
%183 = getelementptr inbounds %struct.sock, %struct.sock* %sk, i64 2, i32 0, i32 12, i32 0, i32 0, i64 3, !dbg !25137
%184 = load volatile i32, i32* %183, align 4, !dbg !25140
%185 = getelementptr inbounds %struct.sock, %struct.sock* %sk, i64 2, i32 0, i32 12, i32 0, i32 0, i64 2, !dbg !25142
%186 = load volatile i32, i32* %185, align 4, !dbg !25145
%sub.i191 = sub i32 %186, %184, !dbg !25147
%cmp.i192 = icmp slt i32 %sub.i191, 0, !dbg !25148
br i1 %cmp.i192, label %if.then.i193, label %lor.rhs.i, !dbg !25148, !prof !14149
lor.rhs.i: ; preds = %zerocopy_rcv_inq
%187 = load volatile i32, i32* %183, align 4, !dbg !25151
%cmp20.i = icmp eq i32 %184, %187, !dbg !25148
br i1 %cmp20.i, label %if.end.i194, label %if.then.i193, !dbg !25153, !prof !15210, !misexpect !14150
if.then.i193: ; preds = %lor.rhs.i, %zerocopy_rcv_inq
call void @lock_sock_nested(%struct.sock* %sk, i32 0) #16, !dbg !25156
%188 = load i32, i32* %185, align 8, !dbg !25157
%189 = load i32, i32* %183, align 4, !dbg !25158
%sub24.i = sub i32 %188, %189, !dbg !25159
call void @release_sock(%struct.sock* %sk) #16, !dbg !25160
br label %if.end.i194, !dbg !25161
if.end.i194: ; preds = %if.then.i193, %lor.rhs.i
%inq.0.i = phi i32 [ %sub24.i, %if.then.i193 ], [ %sub.i191, %lor.rhs.i ], !dbg !25135
%cmp25.i = icmp eq i32 %inq.0.i, 0, !dbg !25162
br i1 %cmp25.i, label %land.lhs.true.i, label %tcp_inq_hint.exit, !dbg !25163
land.lhs.true.i: ; preds = %if.end.i194
%skc_flags.i.i = getelementptr inbounds %struct.sock, %struct.sock* %sk, i64 0, i32 0, i32 15, i32 0, !dbg !25166
%190 = load volatile i64, i64* %skc_flags.i.i, align 8, !dbg !25169
%and1.i.i.i = lshr i64 %190, 1, !dbg !25170
%191 = trunc i64 %and1.i.i.i to i32, !dbg !25170
%192 = and i32 %191, 1, !dbg !25170
br label %tcp_inq_hint.exit, !dbg !25170
tcp_inq_hint.exit: ; preds = %if.end.i194, %land.lhs.true.i
%call636195 = phi i32 [ %192, %land.lhs.true.i ], [ %inq.0.i, %if.end.i194 ]
%inq = getelementptr inbounds %struct.tcp_zerocopy_receive, %struct.tcp_zerocopy_receive* %zc, i64 0, i32 3, !dbg !25171
store i32 %call636195, i32* %inq, align 8, !dbg !25172
br label %zerocopy_rcv_out, !dbg !25173
zerocopy_rcv_out: ; preds = %if.end621, %tcp_inq_hint.exit
%tobool637 = icmp eq i32 %call622, 0, !dbg !25175
br i1 %tobool637, label %land.lhs.true638, label %cleanup644, !dbg !25177
land.lhs.true638: ; preds = %zerocopy_rcv_out
br i1 %cmp3.i.i181212, label %if.then.i.i39, label %if.end12.i.i42, !dbg !25182, !prof !14149, !misexpect !14150
if.then.i.i39: ; preds = %land.lhs.true638
br i1 %180, label %if.else.i.i41, label %if.then7.i.i40, !dbg !25183, !prof !15638
if.then7.i.i40: ; preds = %if.then.i.i39
call void (i8*, ...) @__warn_printk(i8* getelementptr inbounds ([38 x i8], [38 x i8]* @.str.16, i64 0, i64 0), i32 24, i64 %conv617209) #16, !dbg !25187
call void asm sideeffect "...", "i,i,i,i,~{dirflag},~{fpsr},~{flags}"(i8* getelementptr inbounds ([30 x i8], [30 x i8]* @.str.17, i64 0, i64 0), i32 127, i32 2305, i64 12) #14, !dbg !25188, !srcloc !24631
br label %copy_to_user.exit47, !dbg !25189
if.else.i.i41: ; preds = %if.then.i.i39
call void @__bad_copy_from() #16, !dbg !25190
br label %copy_to_user.exit47, !dbg !25190
Here, `%177` feeds into `%178`, which is a PHI node. That feeds into `%179`, which includes it twice and another value that's `true` from another block. So this is the start of the badness. I don't know why that value's there.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D75799/new/
https://reviews.llvm.org/D75799
More information about the llvm-commits
mailing list