[PATCH] D12066: Introduce the ScopExpander as a SCEVExpander replacement

[Tobias Grosser via llvm-commits]

>>> sanjoy added inline comments.
>>>
>>> ================
>>> Comment at: lib/Support/ScopHelper.cpp:292
>>> @@ +291,3 @@
>>> +
>>> +    Value *LHS = Expander.expandCodeFor(LHSScev, E->getType(), StartIP);
>>> +    Value *RHS = Expander.expandCodeFor(RHSScev, E->getType(), StartIP);
>>> ----------------
>>> I don't grok how Polly uses SCEVs, but won't `LHS` be identical to `Inst->getOperand(0)` here?  Likewise for `RHS` and `Inst->getOperand(1)`?  Why can't this function just `return E->getValue()` (after which `ScopExpander::expandCodeFor` should become equivalent to `SCEVExpander::expandCodeFor`)?
>>
>> Sorry, I meant "Why can't this function just `return E;`"
> Because it the SCEVExpander will __not__ generate any code for an
> existing SDiv/SRem instruction but just use it. We want however to
> generate new code if the instruction is in the analyzed region but we
> generate code outside/infront of that region. Hence, we generate the
> code for the SDiv/SRem operands in front of the analyzed region and then
> create a new SDiv/SRem operation there too.

Or saying this with different words. We are expanding a SCEV expression at
a location higher up in the dominance tree as the location for which
it was computed (in the preheader of a large loop-nest, e.g.). As a result,
certain SCEVUnknown components of this expression are not yet defined.
To resolve this issue, we recompute these SCEVUnknown components
(and everything else they require) while expanding them.

Best,
Tobias