Improve insertion location for "hoistable" instructions (#1593)

The Slang IR builder has a notion of "hoistable" instructions, which are basically those instructions that represent a pure side-effect-free operation on their operands, and which can and should be deduplicated. Most types are "hoistable" instructions.

In order to make deduplication of hoistable instructions work, we need to emit them at the right location. Consider if we had:

```hlsl
void myFunc<T>(...)
{
    if(someCondition) {
        vector<T, 4> a = ...;
        ...
    } else {
        vector<T, 4> b = ...;
    }
}
```

The IR instruction that represents `vector<T,4>` can't be inserted at the global scope, because then the parameter `T` would not be visible to it. That instruction also shouldn't be inserted into the same block that declares `a`, because then the instruction itself wouldn't be visible at the point where `b` is declared.

The IR builder already has logic to pick the right parent instruction. In the example given, the IR instruction for `vector<T,4>` should be inserted into the body of the IR generic, but outside of the IR function that represents `myFunc`.

The problem this change fixes is that while the logic was picking the *parent* for a hoistable instruction correctly, it wasn't putting much care into pick the insertion *location*. The existing strategy amounted to:

* If the IR builder was set with an insertion location inside the chosen parent, then use that insertion location

* Otherwise, insert at the end of the chosen parent

Neither of those options is perfect. Either could lead to an instruction being inserted after one of its uses, and the second option could even lead to a type being inserted *after* the `return` instruction in a function/generic, which violates another structural invariant of our IR (that every block must end with a terminator, and terminators must only appear at the end of blocks).

This change updates the rules as follows:

* If the type of the instruction being created, or any of its operands are in the chosen parent, then insert immediately after whichever of those instructions is last in that parent.

* Otherwise, insert before the first non-decoration, non-parameter child of the chosen parent

The combined effect of these two rules is now that we insert any hoistable instruction as early as we can in its parent, without violating the structural validity rules.

(One small exception to these rules is that if the parent is the module then we don't worry about ordering and just insert at the end, since order-of-declaration isn't significant at module scope in our IR)

All of our existing tests work with this new behavior, although there could conceivably be future cases that lead to complicated breakage. For example, if a pass looks at the first "ordinary" instruction in a block and saves it to use as an insertion point for parameter, and then proceeds to manipulate code in the block before going back and inserting parameters at the chosen location, there is a chance that a hoistable instruction might have been inserted before the chosen insertion point, leading to a parameter being inserted after an ordinary instruction. In general, though, code that works like that would already be playing a dangerous game in that it is manipulating instructions in a block while assuming the first instruction will remain fixed.

This change is currently just a refactor, but the underlying issue surfaced as a bug when I made other changes in a feature branch.

0 files changed, 0 insertions, 0 deletions