[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;
> 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
// Convert source pointers to integers, which can be manipulated.
> 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