vsk marked an inline comment as done.
vsk added a comment.
In https://reviews.llvm.org/D29369#665878, @filcab wrote:
> Why the switch to `if` instead of a fully-covered switch/case?
It lets me avoid repeating two function calls:
switch (CGF.getLangOpts().getSignedOverflowBehavior()) {
case LangOptions::SOB_Defined:
return Builder.CreateMul(Ops.LHS, Ops.RHS, "mul");
case LangOptions::SOB_Undefined:
if (CGF.SanOpts.has(SanitizerKind::SignedIntegerOverflow) &&
!CanElideOverflowCheck(CGF.getContext(), Ops))
return EmitOverflowCheckedBinOp(Ops);
return Builder.CreateNSWMul(Ops.LHS, Ops.RHS, "mul");
case LangOptions::SOB_Trapping:
if (CanElideOverflowCheck(CGF.getContext(), Ops))
return Builder.CreateNSWMul(Ops.LHS, Ops.RHS, "mul");
return EmitOverflowCheckedBinOp(Ops);
}
================
Comment at: test/CodeGen/ubsan-promoted-arith.cpp:56
+
+// Note: -SHRT_MIN * -SHRT_MIN can overflow.
+//
----------------
filcab wrote:
> Nit: Maybe `USHRT_MAX * USHRT_MAX` is more understandable?
Yes, I'll fix this.
================
Comment at: test/CodeGen/ubsan-promoted-arith.cpp:99
+// CHECK-LABEL: define signext i8 @_Z4rem3
+// rdar30301609: ubsan_handle_divrem_overflow
+char rem3(int i, char c) { return i % c; }
----------------
filcab wrote:
> Maybe put the rdar ID next to the FIXME? Like this it looks like you might
> have written that instead of `CHECK` by mistake.
I placed the rdar ID here to communicate that the line should become a CHECK
line once the bug is fixed. It will go away with D29437.
https://reviews.llvm.org/D29369
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
