[PATCH] D45308: [IPRA] Do not collect register usage information on functions that can be derefined

[Kit Barton via Phabricator via llvm-commits]

kbarton added a comment.

In https://reviews.llvm.org/D45308#1071185, @vivekvpandya wrote:

> Change looks good to me. But please fix the new test case as suggested by @nemanjai 
>  In this new test do you see improvement in register allocation due to IPRA? If test-case does not show any benefit due to IPRA then it is not suitable test for this patch.
>  Also I hope this patch is also tested with other architecture which have LIT based test-cases for IPRA. It is very unlikely that such test-case will have function with linkage type which return true for isInterposableLinkage() but if such case is there then this change may break such tests.


This test doesn't show an improvement due to IPRA, because it is testing to ensure IPRA is not run on the bar function because it is marked as weak_odr. Without this fix, IPRA would cause the initialization of r3 and r4 between the calls to bar (CHECKS on line 19 and 20 of the test case) to be removed because bar does not clobber those registers. I have other tests that show the benefit of IPRA that I will include in the patch to enable this on PowerPC.

Yes, this patch has been tested with all of the lit tests, including SystemZ and X86 and there were no failures.

In https://reviews.llvm.org/D45308#1083887, @jonpa wrote:

> With this patch, any called function which is not "definition exact" keeps the unmodified regmask on the call instruction, which assumes that the callee saved registers are saved if modified by callee.
>
> I therefore think the check !F.hasLocalLinkage() in isSafeForNoCSROpt() should be replaced with !F.isDefinitionExact(), since the former check only covers a subset of the second one.
>
> Otherwise, the saving of the CSRs might be skipped without this being reflected in the call regmask in caller, right?






================
Comment at: llvm/test/CodeGen/PowerPC/ipra-odr.ll:1
+; RUN: llc -mtriple=powerpc64le-unknown-linux-gnu -enable-ipra < %s | FileCheck %s
+;
----------------
nemanjai wrote:
> Can we make the checks stronger with `-print-regusage`?
I've updated the testcase with -print-regusage. Will upload a new diff soon. 


================
Comment at: llvm/test/CodeGen/PowerPC/ipra-odr.ll:11
+entry:
+b  %add = add nsw i64 %b, %a
+  store i64 %add, i64* @x, align 4
----------------
nemanjai wrote:
> I imagine this stray `b` character will cause test case failures.
Yes, I don't know how that got there. Fixed.


https://reviews.llvm.org/D45308