[PATCH] D54517: [CGP] Limit Complex Addressing mode by number of BasicBlocks to traverse

Serguei Katkov via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Nov 26 18:25:43 PST 2018


skatkov added a comment.

In D54517#1308063 <https://reviews.llvm.org/D54517#1308063>, @john.brawn wrote:

> Looks OK as a fix for the reported bug, with one minor nitpick. Taking a look at PR39625 it looks like there's an underlying problem where we insert a load of useless placeholders, e.g. for
>
>   declare void @otherfn()
>  
>   define i32 @fn(i32* %arg1, i32* %arg2) {
>   entry:
>     %gep1 = getelementptr i32, i32* %arg1, i32 4
>     %gep2 = getelementptr i32, i32* %arg1, i32 8
>     br i1 undef, label %a1, label %b1
>  
>   a1:
>     call void @otherfn()
>     br label %middle
>  
>   b1:
>     call void @otherfn()
>     br label %middle
>  
>   middle:
>     br i1 undef, label %a2, label %b2
>  
>   a2:
>     call void @otherfn()
>     br label %end
>  
>   b2:
>     call void @otherfn()
>     br label %end
>  
>   end:
>     %phi = phi i32* [ %gep1, %a2 ], [ %gep2, %b2 ]
>     %val = load i32, i32* %phi, align 4
>     ret i32 %val
>   }
>
>
> we insert a total of 9 placeholders, but actually we need only one and the rest get deleted later. So I think there's something that could be done to improve that, but that's no reason not to do this fix.


This is exactly an improvement I wrote in the comment above...


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

https://reviews.llvm.org/D54517





More information about the llvm-commits mailing list