[PATCH] D13774: [CodeGen] Mark setjmp/catchret MBBs address-taken

Joseph Tremoulet via llvm-commits llvm-commits at lists.llvm.org
Fri Oct 16 15:07:12 PDT 2015


JosephTremoulet added a comment.

> I'd actually like this transform to work,


In the case that we hit in the bug, we actually get the same codegen despite the MBB being marked address-taken:

- Tail-merge can still reduce the address-taken block to a jmp
- We can still position the address-taken block before its succ, leaving it empty
- We just can't remove the empty block anymore, so we use a different label for the same codegen

Of course, I agree we wouldn't always be able to get the same codegen.  I believe the setjmp testcase added here left around a block with a jmp, owing I guess to the successor having multiple preds...

> I don't want WinEH to get entangled with BlockAddress support, which is what hasAddressTaken is for.


I was actually trying to be careful *not* to use BlockAddress, so that nothing is address-taken at the IR level.  I take it you're saying that distinction isn't helpful?

Happy to consider alternatives:

1. BranchFolding knows how to rewrite jump tables; perhaps something patterned after that to allow it to rewrite the lea/mov for a catchret would be better?  I considered that but since the lea/mov is in code and the jump tables are data it seemed like it would be a harder problem.
2. I'm not sure I understand why we need to split out the lea/mov at PEI.  Could we persist a CATCHRET with explicit target block through to X86MCInsrtLowering, and there find the appropriate place to stick the lea/mov?



> My idea was to create a pseudo instr during frame lowering that defs EAX/RAX and takes no operands. During X86MCInstrLowering, we would scan forwards to the CATCHRET terminator, get the destination MBB operand, and print its symbol.


Since the same catch could have multiple catchrets targeting different successors, I'd be worried about the pseudos for different targets getting merged together somehow and then we wouldn't be able to expand the merged pseudo to something correct.  Does that sound right, or am I misunderstanding?

Thanks.


http://reviews.llvm.org/D13774





More information about the llvm-commits mailing list