[PATCH] D61934: [SCEV] Use wrap flags in InsertBinop

Sam Parker via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Jun 6 02:09:14 PDT 2019


samparker marked an inline comment as done.
samparker added inline comments.


================
Comment at: test/Transforms/LoopIdiom/basic.ll:49
 ; CHECK: %[[BaseBC:.*]] = bitcast i16* %Base to i8*
-; CHECK: %[[Sz:[0-9]+]] = shl i64 %Size, 1
+; CHECK: %[[Sz:[0-9]+]] = shl nuw i64 %Size, 1
 ; CHECK: call void @llvm.memset.p0i8.i64(i8* align 2 %[[BaseBC]], i8 0, i64 %[[Sz]], i1 false)
----------------
nikic wrote:
> This nuw flag seems dubious. Or maybe the whole transform is dubious. It's not immediately clear to me why it would be illegal to have a loop that clears a whole address space using say i16s and size half the address space. In that case this memset formation will go from clearing the address space to setting zero bytes. With the additional nuw flag it will be UB instead. But I don't think it's UB to pass a large size to this function in the first place. Am I wrong about that?
I agree, it looks wrong to me. I can't see how it's safe to double 'Size'.


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D61934/new/

https://reviews.llvm.org/D61934





More information about the llvm-commits mailing list