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

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