[PATCH] D48066: Add one more No-alias case to alias analysis.

[Hal Finkel via Phabricator via llvm-commits]

hfinkel added a comment.

I think that this optimization is going to be very fragile in practice without also handling the case where both sides look like (m-idx). Nevertheless, it's good to make AA changes in minimal small increments. Let's try this single case for now.

Even with this, however, regarding this:

> It seems the 'computeKnownBits' does not handle non-constant values well.
> 
> For example,
> 
>   define void @test(i32 %idx) {
>   entry:
>    %sub = sub nsw i32 3, %idx
>    
> 
> It fails to get Zero and One from %idx.

I'm not sure you mean? In this case, nothing is known about idx, so what would it get?

However, if you do something like:

   ... @test(i32 %i) {
    %idx = and i32 %i, 3
    %sub nsw i32 3, %idx
  }

then computeKnownBits will know something about the bits of idx.



================
Comment at: lib/Analysis/BasicAliasAnalysis.cpp:1812
+  if (const BinaryOperator *BOp = dyn_cast<BinaryOperator>(V1)) {
+    if (BOp->getOpcode() != Instruction::Sub) {
+      return false;
----------------
Don't need { } here.


================
Comment at: lib/Analysis/BasicAliasAnalysis.cpp:1816
+
+    if (BOp->getOperand(1) != V2) {
+      return false;
----------------
Don't need { } here.


================
Comment at: lib/Analysis/BasicAliasAnalysis.cpp:1822
+    // Check odd number.
+    if ((KnownLHS.One.countTrailingZeros()) == 0) {
+      return true;
----------------
Don't need { } here. Also, is there an extra set of parenthesis here?


================
Comment at: lib/Analysis/BasicAliasAnalysis.cpp:1879
+    // offset2 = index;
+    if (V1Size != V2Size) {
+      return false;
----------------
Don't need { } here.


https://reviews.llvm.org/D48066