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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
