[PATCH] D19884: Fix a bug when hoist spill to a BB with landingpad successor

[Quentin Colombet via llvm-commits]

qcolombet added a comment.

Hi Wei,

I’ll second Matthias that we should not need the full split kit here and refactoring should be the way to go.
I would however like to keep the caching mechanism at least in split kit. So, like pretty much Matthias said:

- Introduce a utility function to query the last split point.
- Introduce a class that does the caching on top of that utility function. (That could even be an analysis pass, but that is probably overkill.)

That way if we have users that do not want the caching mechanism (for hoist spill?) they have a direct access to the utility function, otherwise they use the class (for split kit).

Couple of comments on the test case:
I believe this is a test case reduced by bug point, and I believe you can make it smaller and more robust to other changes. Basically come-up with a simple CFG pattern where the spill hoisting is triggered (a diamond should be enough, right), add a lpad at the strategic point, then add a few inlineasm nop with side effects clobbering all the registers (or at least a lot o them) to create high register pressure points where need be.

I know this is a hassle but there are reasons why I am asking this:

1. This kind of test cases happens to work because we are not removing undef branches and such. (We could imagine this won’t stand in the future.)
2. When the test fails because of regalloc (or other) changes, this is pretty much impossible to keep the coverage they were providing and we unfortunately just remove them :(.

Cheers,
-Quentin


================
Comment at: test/CodeGen/X86/hoist-spill-lpad.ll:8
@@ +7,3 @@
+;
+; CHECK-LABEL: _Z5CheckIZ4mainE3$_0EvT_:
+; CHECK: movq	%r14, 16(%rbx)          # 8-byte Spill
----------------
Could we use simpler name for the function :).


Repository:
  rL LLVM

http://reviews.llvm.org/D19884