[PATCH] D13378: [SCEV] Recognize simple br-phi patterns

Sanjoy Das via llvm-commits llvm-commits at lists.llvm.org
Fri Oct 2 14:43:55 PDT 2015



Mehdi Amini wrote:
 >> On Oct 2, 2015, at 2:02 PM, Sanjoy 
Das<sanjoy at playingwithpointers.com>  wrote:
 >>
 >> sanjoy added a comment.
 >>
 >> In http://reviews.llvm.org/D13378#258915, @joker.eph wrote:
 >>
 >>> Hi Sanjoyd,
 >>>
 >>> Thanks for the clarification on the Constant Select, isn't it 
something that could be handled as well? Especially with 
http://reviews.llvm.org/D13390 coming?
 >>
 >> I'm not sure there is much value in optimizing `ConstantExpr`s in 
SCEV, because they're compile time constants anyway (though they may 
need to remain symbolic till link time).  I'd just wait till we see a 
case where it matters in practice.
 >
 > In my mind it is not about purely “optimizing” this case in SCEV 
alone. I was wondering about other transformations (loop for instance) 
that are relying on the result of SCEV analysis to do more powerful 
optimizations. I’m not completely sure about how SCEV work but I feel 
cases like this that are not handled prevent a recurrence from being 
inferred in a loop for instance.
 > Canonicalizing the IR solves the issue "most of the time”, but as 
shown in http://reviews.llvm.org/D13390 it is not always possible to 
keep the IR “canonical” at the time you need a SCEV result.
 > I agree that in general we’d better have some motivational example to 
drive this, but sometimes the best way to figure out if something can 
have an impact is to implement it and measure :)
 > I may try to run some benchmarks tonight to see if this case come up 
in practice, feel free to commit this without waiting for results from 
my side since this is just about some (highly) hypothetical speculations.

As far as I can tell, `ConstantExpr` s are canonicalized on creation,
so I suspect the kind of IR canonicalization problems you're seeing
won't happen with `ConstantExpr` s (but please verify this).

I think the only reason why we cannot manipulate `ConstantExpr` s as
`SCEVConstant` instances is because a `SCEVConstant` has to wrap a
`ConstantInt`, and nothing else.  However, if we move towards adding a
new SCEV node type, `SCEVConstantExpr` that wraps arbitrary constants,
then we'd be able to treat arbitrary `ConstantExpr` s as constants and
not `SCEVUnknown` like they are today.  OTOH, I don't know how much
optimization value we'll get out of having constants that we cannot
direct inspect the numerical value of.

-- Sanjoy

 >
 > Thanks!
 >
 > —
 > Mehdi
 >


More information about the llvm-commits mailing list