[PATCH][PR16882] Ignore noreturn definitions when setting isPhysRegUsed.

Jim Grosbach grosbach at apple.com
Tue Sep 24 11:58:28 PDT 2013


Seems OK. I wonder if it’s worth it calculating the set of no-return instructions ahead of time. Looking for the common case of there not being any at all seems a reasonable optimization, but in the case where we have one or more, just checking directly from the iterator rather than a lookup in a SmallPtrSet<> will probably be just as fast and be a bit simpler code?

-Jim

On Sep 24, 2013, at 11:51 AM, Eric Christopher <echristo at gmail.com> wrote:

> Seems sane to me, Jim was one of the last people in here so he could
> probably review as well.
> 
> -eric
> 
> On Tue, Sep 24, 2013 at 10:48 AM, Quentin Colombet <qcolombet at apple.com> wrote:
>> Ping^3?
>> 
>> -Quentin
>> 
>> On Sep 17, 2013, at 10:54 AM, Quentin Colombet <qcolombet at apple.com> wrote:
>> 
>> Ping^2?
>> 
>> -Quentin
>> 
>> On Sep 10, 2013, at 10:54 AM, Quentin Colombet <qcolombet at apple.com> wrote:
>> 
>> Ping?
>> 
>> -Quentin
>> 
>> On Sep 3, 2013, at 2:16 PM, Quentin Colombet <qcolombet at apple.com> wrote:
>> 
>> Hi,
>> 
>> The proposed patch fixes PR16882:
>> http://llvm.org/bugs/show_bug.cgi?id=16882.
>> PR16882 reports a poor code generation issue where we were saving the link
>> register before a call to a noreturn function, whereas it is not needed.
>> 
>> Thanks for your reviews.
>> 
>> ** Details **
>> 
>> PEI inserts a save/restore sequence for the link register, according to the
>> information it gets from the MachineRegisterInfo.
>> MachineRegisterInfo is populated by the VirtRegMap pass.
>> This pass was not aware of noreturn calls and was registering the
>> definitions of these calls the same way as regular operations.
>> 
>> 
>> ** Proposed Solution **
>> 
>> Modify VirtRegPass so that it does not set the isPhysRegUsed information for
>> registers only defined by noreturn calls.
>> The rational is that a noreturn call is the "last instruction" of the
>> program (if it returns the behavior is undefined), so everything that is
>> defined by it cannot be used and will not interfere with anything else.
>> Therefore, it is pointless to account for then.
>> 
>> 
>> ** Experiments **
>> 
>> I have made functional tests on x86_64 with O3 -fomit-frame-pointer on the
>> llvm test-suite + SPEC.
>> I did the same for ARMv7s.
>> 
>> I have attached the performance numbers for ARMv7s (lower is better). These
>> numbers only reports benches where the patch kicks in and the test runs for
>> more than 1s (to avoid noise).
>> The Reference column is LLVM r189613 and Test column is LLVM r189613 + the
>> proposed patch.
>> 
>> Cheers,
>> -Quentin
>> <virtreg.svndiff>
>> <noreturn_perf.txt>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>> 
>> 
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>> 
>> 
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>> 
>> 
>> 
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>> 





More information about the llvm-commits mailing list