[llvm-dev] [SCEV] inconsistent operand ordering

[Chawla, Pankaj via llvm-dev]

Thanks for the helpful reply!

I see that we are trying to keep ScalarEvolution stable around instruction ordering. My suggestion would be to not restrict the fix by only recursing on the first operand. 

By "dominator logic"  I meant that if all other 'cheap' checks fail, we should decide by walking the dominator tree to see which instruction's basic block is encountered first from the function entry block. But we obviously cannot use this if we want ScalarEvolution to be stable around 'simple' CFG changes.

Thanks,
Pankaj


-----Original Message-----
From: Sanjoy Das [mailto:sanjoy at playingwithpointers.com] 
Sent: Tuesday, October 18, 2016 12:01 PM
To: Chawla, Pankaj
Cc: llvm-dev at lists.llvm.org
Subject: Re: [llvm-dev] [SCEV] inconsistent operand ordering

Hi Pankaj,

Chawla, Pankaj wrote:
> Thanks for fixing this Sanjoy!
>
> I do have a few questions/suggestions on the fix if you don't mind.
>
> 1) Would this work correctly if the values are call instructions with 
> no operands, like this- %a = foo() %b = bar()
>
> 2) From the way this function is set up, it looks like the emphasis is on saving compile time by trading off robustness. Is compile time such a big concern here that we want to fix this one test case at a time? We can perhaps use dominator logic to fix this once and for all.

For (1), in addition to the recursion, I think we'll need to differentiate between global variables.

For (2), I'm not quite sure what you mean by "dominator logic".

I agree with you generally: within reason, we should try to canonicalize as much as possible.

> 3) When both instructions are in the same basic block, iterating the bblock and returning based on the instruction which is encountered first might be better.

This was my first impulse, but I'll have to think about whether:

  a. It makes SCEV too fragile around instruction ordering -- we don't want divergent behavior triggered by seemingly innocent ordering differences.

  b. What obligations will then exist for passes that move instructions with a basic block, since that can now change the definition of canonical SCEV expressions.

-- Sanjoy