[PATCH] Simplify and correct landing pad lowering

[Jakob Stoklund Olesen]

On Jul 3, 2013, at 4:27 PM, Stephen Lin <swlin at post.harvard.edu> wrote:

> Wow, I'm surprised this bug never caused any issues earlier. Was there
> something in practice that usually kept the scheduling from clobbering
> the physical registers?

Landing pads don’t normally contain division or multiplication instructions.

(And our test coverage of landing pad code isn’t stellar.)

If there are only virtual register defs between the landing pad entry and the COPY instructions, the register allocator can figure out the live ranges for the %rax and %rdx registers, so in those cases it still works. It’s the hardwired physreg defs that can cause problems.

/jakob


> 
> On Wed, Jul 3, 2013 at 4:17 PM, Jakob Stoklund Olesen <stoklund at 2pi.dk> wrote:
>> Hi,
>> 
>> Please review these patches which fix a problem with our landing pad lowering and remove some unnecessary abstraction.
>> 
>> DWARF-style landing pads receive two arguments in registers: The exception pointer (%rax) and the exception selector (%rdx). Currently, these two arguments are represented by the abstract ISD::EXCEPTIONADDR and ISD::EHSELECTOR SDNodes. All in-tree targets legalize these nodes by expanding them to CopyFromReg nodes reading the corresponding physregs (%rax and %rdx).
>> 
>> This doesn’t actually work correctly because there is no dependency preventing the %rax and %rdx registers from being clobbered before the CopyFromReg nodes are scheduled. A landing pad with a DIV instruction could easily do that:
>> 
>> BB#4: derived from LLVM BB %lpad.i.i, EH LANDING PAD
>>    Live Ins: %EAX %EDX
>>    Predecessors according to CFG: BB#3
>> ..
>>        DIV %EAX<imp-def>, %EDX<imp-def>; Live-in values clobbered.
>> ..
>>        %vreg28<def> = COPY %EAX<kill>; GR32:%vreg28
>>        %vreg29<def> = COPY %EDX<kill>; GR32:%vreg29
>> 
>> 
>> These patches change the handling of landing pad arguments to look more like the handling of normal function arguments. The live-in physical registers are immediately copied into virtual registers at the top of the basic block. The CopyFromReg nodes are changed to read the virtual registers instead of reading the live-in physregs directly.
>> 
>> I also went ahead and removed the EXCEPTIONADDR, EHSELECTOR, and LSDAADDR ISD opcodes. They don’t seem to provide a useful abstraction in addition to the TLI.setExceptionPointerRegister() / TLI.setExceptionSelectorRegister() functions, and all targets use the default ‘Expand’ legalization anyway.
>> 
>> Thanks,
>> /jakob
>> 
>> 
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits