In D158664#4611498, @davidxl wrote:

I assume this is with instrumentation based FDO. Why are there functions without profile?

This also fixed the problem we found in our build. The code looks good. Thanks.

LGTM. Thanks!

LGTM. Thanks for the quick fix!

Our SPEC build on PowerPC failed due to this patch. Following PR (gd.ll) is extracted from gcc_r build:

Rewrite getMallocType - only look at the bitcast feeding into the store (value or pointer) for malloc call, and no longer return nullptr.

In D107574#2929921, @efriedma wrote:

In D107574#2929796, @scui wrote:

It looks to me the IR code will be sub-optimal for some cases if we simply return Type i8* for getMallocType.

Will it? Almost all optimizations can deal with bitcasts of global variables without getting confused.

I could be wrong. One case I can imagine is to set initial value, for example, an i32 value for [4 x i8] global.

In D107574#2929159, @efriedma wrote:

If you going to mess with this code, can you just kill off all the malloc use type heuristics, and just unconditionally create an [i8 x n] array? The existing type sniffing code won't work with opaque pointers anyway.

To address review comments

In D106589#2920763, @saugustine wrote:

This patch caused the following error for me:

error: Explicit load/store type does not match pointee type of pointer operand (Producer: 'LLVMgoogle3-trunk' Reader: 'LLVM google3-trunk')

I'm working on a reduced test case, but it might take a while. If the error sparks any ideas, it would be good to hear them.

This is to address comments: (1) fix for the ConstantExpr case in allUsesOfLoadedValueWillTrapIfNull, (2) Consider multiple uses of ConstantExpr for the loop to traverse uses of GV in OptimizeGlobalAddressOfMalloc. The load/store uses are collected in a set first and then loop over the set.

This is to address comments: (1) limit the case for ConstantExpr to only bitcast and gep in allUsesOfLoadedValueWillTrapIfNull, (2) change back using while loop to traverse uses of GV in OptimizeGlobalAddressOfMalloc

Exclude the signed null checks, and correct the handling of UGE.

Add two test cases - one is to test the icmp condition with non-null value, the other is to test the disablement of the sinking of load to the icmp location.

In D102711#2785937, @efriedma wrote:

I'd like to see a little more test coverage, given all the different cases where the behavior of the transform changed.

This to address the review comments: (1) Refine the code to handle ICmpInst when checking if AllUsesOfValueWillTrapIfNull, (2) Consider the GlobalVariable is initialized when creating StoreInst for the global init bool symbol, (3) Remove the unsafe code sinking when transforming the ICmpInst.

gentle ping

Thanks Roland for the approval. I don't have commit access. Can you help pls to commit this? The author line should be --author="Shimin Cui <scui@ca.ibm.com>".

To Address comments from Roland - fix the comment in def and add missing vec_free in BuildLibCalls.cpp

In D94710#2513352, @RolandF wrote:

Test case?

Thanks Jeeva. I need your help to commit this, I don't have commit access yet. The author line should be --author="Shimin Cui <scui@ca.ibm.com>".

In D88669#2345905, @jdoerfert wrote:

Reverse ping. More comments (nits) and notice that we should merge this, D89830 reimplemented it in the meantime, see my comment there too.

Updated to address review comments: I adopted the use of Log2_32 from D89830, removed the parenthesis comment, and added a TODO comment in the test case.

Rewrite the code using getKnowledgeValidInContext. The code is much cleaner than the previous version, but the result computed is less accurate if the size of the alignment bundle is three. I'll leave the improvement for the future, together with the improvement of value tracking of GEP (an example is that, with the future improvement, we can fold away the branch "br i1 %3, ...") in f2 of the added case assume-align.ll).

In D88669#2328334, @efriedma wrote:

What does the third operand of an align bundle mean? It doesn't seem to be documented in LangRef.

Anyone has any comments for this patch? Any comments or suggestions are appreciated.

In D87850#2322762, @efriedma wrote:

LGTM

Do you have commit access? If not, I can commit it for you; what should I put in the git "Author" line?

Remove the added extra cases from the existing test files

Adjust the test cases.

Anyone has any comments for this patch? Any comments or suggestions are appreciated.

Fix previous reverion of 2004-11-27-SetCCForCastLargerAndConstant.ll and PR17073.ll

Revert 2004-11-27-SetCCForCastLargerAndConstant.ll and PR17073.ll