Test approach to handling clobbering llvm.eh.selector return

Hi Duncan,

I hacked together a version of DwarfEHPrepare.cpp which tries to deal with the ordering of llvm.eh.exception and llvm.eh.selector.
The hacked is contained within the attached patch.

Motivation:

I recently created a decent amount of hand coded IR (via the llvm C++ API). In order to help me runtime debug the code,
I created automatic constructors which would trace entries into the blocks depending on the setting of a build flag. This means
that the first instruction to every block was a call to fprintf. The trace can be delayed for scenarios such as blocks with phi nodes.
For landing pads however, I ran into the issue where the runtime call to fprint seemed to affect the same register used as that used
by llvm.eh.selector's return. When tracing was turned on then, I got garbage for llvm.eh.selector's return. As you stated in a
previous email this should be fixed in a similar manner as that given for llvm.eh.exception in DwarfEHPrepare. You also stated
there were issues, which I believe I partially understand (see bottom).

Its goals are:

1) To move llvm.eh.selector calls immediately after llvm.eh.exception calls within landing pads
2) To force the exception argument of llvm.eh.selector to come from the return of the preceding llvm.eh.exception call.
3) To create and move loads of llvm.eh.selector immediately after newly created loads of llvm.eh.exception in non-landing pads.
4) To add the relevant stores (after steps 1 and 2 are finished for both llvm.eh.exception and llvm.eh.selector) for llvm.eh.selector
5) To include loads and stores of llvm.eh.selector to the register promotion optimization.
6) To include statistics for llvm.eh.selector call "moves".

The following paths were manually tested:

1) The movement of llvm.eh.exception calls (pre-existing functionality), and llvm.eh.selector within landing pads.
2) The above path with the additional change of llvm.eh.selector's exception argument as described in goal 2.

The following paths were NOT tested:

1) Non-landing transformations of llvm.eh.selector calls to Loads, along with the required inclusion of stores
2) All the complicated scenarios that these algorithms may have to deal with.

Two implementations, who's choice is determined by hard coded preprocessor if, are given:

1) Following the pre-existing code patterns for the movement of landing pad llvm.eh.exception calls,
  llvm.eh.selector calls (when necessary), are cloned and inserted at the correct position with
  the old llvm.eh.selector call "removed". In this implementation a necessitated changed of the new
  llvm.eh.selector's exception argument results in the old instance of this argument not being removed
  even if its use drops to zero. Because the old llvm.eh.selector call is not replace by the new
  llvm.eh.selector call until after the exception argument is changed (if necessary), the old exception
  argument will have at least 1 use at the point where the "new" exception argument is applied to the
  new llvm.eh.selector call. The code, at this point, could remove the exception argument from the old
  llvm.eh.selector call, but this treatment would not follow the replace semantics of pre-existing code.
2) The llvm.eh.selector is instead moved to the correct position (using llvm.Instruction.moveBefore(...)), with
  any necessary exception argument change. This version required a change in the control flow and is
  therefore larger addition to DwarfEHPrepare.

Problems:

Although the patch deals with the llvm.eh.selector's exception argument, it does not know how to deal with
its other arguments. This would not be a problem for say type infos, and I believe the personality function since
these arguments must be direct references to Globals. However I'm unclear on filters, as I have never used
them. Regardless I ignored this issue since you have a full understanding as to whether this is an issue or
not, and as to how to fix it if it is relevant.

Some of the relevant changes are marked with commented a GMV:, which would be removed for the real version.
In addition I have not yet added comments, preferring to delay until and if you think this fix is worth anything.

Hopefully this effort is not too obfuscated for your use.

Garrison

DwarfEHPrepare.patch (15.5 KB)

Hi Garrison,

I hacked together a version of DwarfEHPrepare.cpp which tries to deal with the ordering of llvm.eh.exception and llvm.eh.selector.
The hacked is contained within the attached patch.

it looks like you tried to copy the code for eh.exception. There are two
problems with this: (1) the eh.exception code really needs to be rewritten
to make use of the new SSAUpdator (then all the mucking around with domtree
can go away; it's quite tricky to preserve dominfo when updating the IR, and
the whole dom approach is inefficient), and (2) the eh.selector case is much
more complicated because selectors carry type infos (and filters) around with
them. Sadly, you can just move selectors to the landing pad(s) their exception
objects come from, since you might end up with multiple selectors there, and
because the order of type infos matters, you can't just concatenate the type
info lists. It's worse for filters because these have extra, hidden, semantics
that are enforced by the personality function.

2) To force the exception argument of llvm.eh.selector to come from the return of the preceding llvm.eh.exception call.

This sounds bogus. llvm.eh.selector compares it's exception argument with the
list of type infos, and returns an appropriate index. There is no connection
with "the current exception" in general, or the preceeding llvm.eh.exception
call in particular. This is one of the problems dealing with eh.selector when
the call is far from the landing pad where the exception was caught. That said,
this doesn't work properly right now anyway.

5) To include loads and stores of llvm.eh.selector to the register promotion optimization.

The register promotion should be reworked using SSAUpdator, as I mentioned
above.

Although the patch deals with the llvm.eh.selector's exception argument, it does not know how to deal with
its other arguments. This would not be a problem for say type infos, and I believe the personality function since
these arguments must be direct references to Globals. However I'm unclear on filters, as I have never used
them. Regardless I ignored this issue since you have a full understanding as to whether this is an issue or not, and as to how to fix it if it is relevant.

See above.

Ciao,

Duncan.

Bummer, I had a feeling I was only solving the test case that pertained to my problem: the clobbering of the
register, used by the selector's return, by intervening instructions (those between the llvm.eh.exception call
and the llvm.eh.selector call). The forcing of the selector's exception argument got rid of a bitcast problem,
and I imagined any other potential operation, that manifested itself after the reorder. So now I've got a hacked
version of DwarfEHPrepare that only works for my use case. :slight_smile: I guess it is time to look at SSAUpdator even
though this will not fix the selector argument problem.

So my new conservative IR generation resolution is:

1) Execute llvm.eh.exception as the first instruction of the landing pad.
2) Execute llvm.eh.selector as the second instruction of that landing pad.

Although llvm currently allows more flexibility, holding to those two
resolutions will potentially avoid the trashing of llvm.eh.selector's
return.

Thanks for the response

Garrison