[PATCH] D156821: [CodeGen] [ubsan] Respect integer overflow handling in abs builtin

Fangrui Song via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Fri Aug 18 13:44:45 PDT 2023


MaskRay reopened this revision.
MaskRay added inline comments.
This revision is now accepted and ready to land.


================
Comment at: clang/lib/CodeGen/CGBuiltin.cpp:1787
+    if (!VCI->isMinSignedValue()) {
+      return EmitAbs(CGF, ArgValue, true);
+    }
----------------
nit: we delete braces in this cascading case

https://llvm.org/docs/CodingStandards.html#don-t-use-braces-on-simple-single-statement-bodies-of-if-else-loop-statements


================
Comment at: clang/test/ubsan/TestCases/Misc/abs.cpp:1
+// REQUIRES: arch=x86_64
+//
----------------
Did you mean compiler-rt/ instead of clang/?

`// REQUIRES: arch=x86_64` is legacy style.

New style uses `// REQUIRES: target={{x86_64.*}}`

Actually, this test should be generic and we should remove REQUIRES.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156821



More information about the cfe-commits mailing list