[PATCH] D25849: [GVN] Prevent load coercion with irregular vector types

Davide Italiano via llvm-commits llvm-commits at lists.llvm.org
Thu Oct 20 18:05:16 PDT 2016

davide added inline comments.

Comment at: lib/Transforms/Scalar/GVN.cpp:721
+  // going to crash later anyway.
+  if (llvm::alignTo(StoreSize, 8) != StoreSize)
+    return false;
reames wrote:
> StoreSize % 8 == 0 would seem to be a more standard way to express this check.
> Also, checking only the StoreSize seems suspect.  Why do we not need to test the load size?
I think checking also the `LoadSize` is sensible.

Comment at: lib/Transforms/Scalar/GVN.cpp:783
          "CanCoerceMustAliasedValueToLoad fail");
   // Convert source pointers to integers, which can be manipulated.
reames wrote:
> Have you looked at what's required to fix this code path?  This should be the only logic protected by the predicate you're modifying.
No, I didn't. Personally I looked at this because I got an internal report of a crash and I bothered to fix it.
The test is generated by a fuzzer so I'm not entirely sure how likely is to hit a situation like this in the wild, but still I thought it was worth fixing. I'm working with Daniel on NewGVN and I'd rather focus my efforts on that rather than fixing the load coercion system of some code I hope will go away in the foreseeable future. 


More information about the llvm-commits mailing list