[PATCH] D45343: [InstCombine] Always remove null check before free

[Roman Lebedev via Phabricator via llvm-commits]

lebedev.ri added a comment.

> Null check before free is currently removed only when we are optimizing for size.
>  This patch removes that limitation since free already handles null pointer passed as argument so null check before free can be easily removed.

I'm seeing "what", but not seeing "why"...
The original https://reviews.llvm.org/rL171762 talked about only doing that when it is known to be profitable.
It might be helpful to explain in the differential's description why it is now
preferable to always do this, not only when optimizing for size.



================
Comment at: test/Transforms/InstCombine/null-check-free.ll:21
+;
+entry:
+  %tmp = alloca i8*, align 4
----------------
So what exactly does that diff change here in the output?
https://godbolt.org/g/CJaEsg i don't see any difference.

This is why it is best to have two separate differentials,
one with just the original testcase, and the other one with the code change + regenerated test output.


================
Comment at: test/Transforms/InstCombine/null-check-free.ll:33
+if.end:
+  %1 = load i8*, i8** %tmp, align 4
+  ret i8* %1
----------------
Hmm, i don't like the test. Does it not end up returning dangling pointer?


https://reviews.llvm.org/D45343