Example:
__attribute__((nonnull,noinline)) char * pinc(char *p) { return ++p; } char * foo(bool b, char *a) { return pinc(b ? 0 : a); }
optimize to
char * foo(bool b, char *a) { return pinc(a); }
Differential D94180
[SimplifyCFG] Optimize CFG when null is passed to a function with nonnull argument xbolva00 on Jan 6 2021, 9:14 AM. Authored by
Details Example: __attribute__((nonnull,noinline)) char * pinc(char *p) { return ++p; } char * foo(bool b, char *a) { return pinc(b ? 0 : a); } optimize to char * foo(bool b, char *a) { return pinc(a); }
Diff Detail
Event Timeline
Comment Actions test missing?
Comment Actions Yeah, I believe passing null to nonnull should not immediately raise UB; it will block useful analyses. The patch is D90529, and I need to push it... Maybe it is time to reduce the number of patches that are still open by me. For the example, I think this InstCombine transformation will work. noundef isn't necessary. %s = select i1 %cond, i8* null, i8* %a call void @foo(i8* nonnull %s) -> call void @foo(i8* nonnull %a) This is correct because For the SimplifyCFG change, noundef is necessary. :) Maybe we can have both SimplifyCFG and InstCombine?
Comment Actions
Yeah, I will do it as a follow up patch.
Comment Actions Why only use GEPs? And it looks to me you only check the offset, doesn't that miscompile: I would have assumed you check for nonnull and noundef, then you ask isKnownNull, and it's UB if all 3 are true. Comment Actions Added test9_gep_mismatch.
We dont have 'isKnownNull' or miss I something? Comment Actions I think I was and am confused. V == C is null at the location the new code is inserted, correct? Is V also equal to I? As I said, I'm confused.
Comment Actions define void @test9_gep_mismatch(i1 %X, i8* %Y, i8* %P) { ; CHECK-LABEL: @test9_gep_mismatch( ; CHECK-NEXT: entry: ; CHECK-NEXT: [[SPEC_SELECT:%.*]] = select i1 [[X:%.*]], i8* null, i8* [[Y:%.*]] ; CHECK-NEXT: [[GEP:%.*]] = getelementptr inbounds i8, i8* [[P:%.*]], i64 0 ; CHECK-NEXT: [[TMP0:%.*]] = call i8* @foo(i8* [[GEP]]) ; CHECK-NEXT: ret void ; entry: br i1 %X, label %if, label %else if: br label %else else: %phi = phi i8* [ %Y, %entry ], [ null, %if ] %gep = getelementptr inbounds i8, i8* %phi, i64 7 call i8* @foo(i8* %gep) ret void } So for this case, Use is call i8* @foo(i8* %gep) I is %gep = getelementptr inbounds i8, i8* %phi, i64 7 and V is a incoming value from phi. We bailt out if V is not constant, it needs to be null value. Comment Actions (Don't wait for me to finish the review)
Comment Actions Generally LGTM, assuming you are fine with my comments. If you want to continue the discussion, we can as well.
Comment Actions Logic looks fine now, though the GEP handling can be made a bit more lax.
|
Why does this pass a pointer to bool, rather than just a bool?