In special cases select instructions can be eliminated by
replacing them with a cheaper bitwise operation. The instance implemented
by this pass is %x=icmp.eq; %y=select %x,%r, null; %z=icmp.eq %y, null;
br %z,true, false ==> %x=icmp.ne; %y=icmp.eq %r,null; %z=or %x,%y; br
%z,true,false. The optimization is global and performed only when all uses
of the select result can be replaced by the select true value. For this
dominator information is used.
Details
- Reviewers
echristo
Diff Detail
Event Timeline
Select Elimination Pass update
Improved readability and addressed review:
- moved code that replaces select into a function replaceSelectWithOr()
- moved the checks for the selection condition compare into functio isChainSelectCmpEQBranch().
- cleared up some clutter as David M. had suggested
Hi Gerolf,
I added a few comments and questions inline.
-Juergen
lib/Transforms/Scalar/SelectElimination.cpp | ||
---|---|---|
80 | Your pass doesn't preserve the CFG? | |
82 | We don't add the function name in the comments anymore. | |
105 | ditto | |
118 | ditto | |
145 | ditto | |
170 | ditto | |
175 | I would add an assert with isa<ICmpInst> and then use just cast<ICmpInst>, since you are not checking the result of the dyn_cast anyways. | |
212 | Why is the iterator incremented in the loop? | |
214 | You could combine this with dyn_cast to one line or you could invert the condition and add a continue to reduce the indentation of the code afterwards. |
lib/Transforms/Scalar/SelectElimination.cpp | ||
---|---|---|
61 | The command-line option should be in the PassManagerBuilder. Rather than return from the runOnFunction, we should not add the pass to the pass manager. |
I integrate the optimization into InstCombine and close this review. @Juergen, Chad, Dave and Chandler - thank you for your review time and comments.
The command-line option should be in the PassManagerBuilder. Rather than return from the runOnFunction, we should not add the pass to the pass manager.