[PATCH] D49492: Run bounds checking sanitizer earlier to make it easier to optimize away its checks.
jgalenson abandoned this revision. jgalenson added a comment. I just committed https://reviews.llvm.org/rL337830, which solves this problem the way eugenis suggested. Repository: rC Clang https://reviews.llvm.org/D49492 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46406: [docs] Fix typos in the Clang User's Manual.
jgalenson created this revision. jgalenson added reviewers: hans, sylvestre.ledru. Repository: rC Clang https://reviews.llvm.org/D46406 Files: docs/UsersManual.rst Index: docs/UsersManual.rst === --- docs/UsersManual.rst +++ docs/UsersManual.rst @@ -1735,11 +1735,11 @@ .. option:: -fprofile-generate[=] The ``-fprofile-generate`` and ``-fprofile-generate=`` flags will use - an alterantive instrumentation method for profile generation. When + an alternative instrumentation method for profile generation. When given a directory name, it generates the profile file ``default_%m.profraw`` in the directory named ``dirname`` if specified. If ``dirname`` does not exist, it will be created at runtime. ``%m`` specifier - will be substibuted with a unique id documented in step 2 above. In other words, + will be substituted with a unique id documented in step 2 above. In other words, with ``-fprofile-generate[=]`` option, the "raw" profile data automatic merging is turned on by default, so there will no longer any risk of profile clobbering from different running processes. For example, Index: docs/UsersManual.rst === --- docs/UsersManual.rst +++ docs/UsersManual.rst @@ -1735,11 +1735,11 @@ .. option:: -fprofile-generate[=] The ``-fprofile-generate`` and ``-fprofile-generate=`` flags will use - an alterantive instrumentation method for profile generation. When + an alternative instrumentation method for profile generation. When given a directory name, it generates the profile file ``default_%m.profraw`` in the directory named ``dirname`` if specified. If ``dirname`` does not exist, it will be created at runtime. ``%m`` specifier - will be substibuted with a unique id documented in step 2 above. In other words, + will be substituted with a unique id documented in step 2 above. In other words, with ``-fprofile-generate[=]`` option, the "raw" profile data automatic merging is turned on by default, so there will no longer any risk of profile clobbering from different running processes. For example, ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D49492: Run bounds checking sanitizer earlier to make it easier to optimize away its checks.
jgalenson created this revision.
jgalenson added reviewers: eugenis, chandlerc.
Herald added a subscriber: cfe-commits.
Running the bounds checking sanitizer earlier makes it easier for other
optimizations to remove the checks it inserts.
While it could also inhibit other optimizations, I ran a few benchmarks and
this did seem to improve performance and code size slightly.
Note that I'm not sure how to hook this up to the new PM, as I couldn't find a
similar hook into the beginning of the pipeline. Are there any suggestions on
what to do about that, or is it fine for this to work just on the old PM?
Repository:
rC Clang
https://reviews.llvm.org/D49492
Files:
lib/CodeGen/BackendUtil.cpp
test/CodeGen/bounds-checking-opt.c
Index: test/CodeGen/bounds-checking-opt.c
===
--- /dev/null
+++ test/CodeGen/bounds-checking-opt.c
@@ -0,0 +1,20 @@
+// RUN: %clang -S -fsanitize=local-bounds -emit-llvm -target x86_64-- %s -o -
| FileCheck -check-prefix=O0 %s
+// RUN: %clang -S -fsanitize=local-bounds -emit-llvm -target x86_64-- -O2 %s
-o - | FileCheck -check-prefix=O2 %s
+// RUN: %clang -S -fsanitize=local-bounds -emit-llvm -target aarch64-- %s -o -
| FileCheck -check-prefix=O0 %s
+// RUN: %clang -S -fsanitize=local-bounds -emit-llvm -target aarch64-- -O2 %s
-o - | FileCheck -check-prefix=O2 %s
+
+extern void fill(int *arr);
+
+// CHECK-LABEL: @f
+int f(int x) {
+ // O0: call {{.*}} @llvm.trap
+ // O2-NOT: call {{.*}} @llvm.trap
+ int foo[1000];
+ fill(foo);
+ int sum = 0;
+ #pragma clang loop vectorize(disable)
+ #pragma clang loop unroll(disable)
+ for (unsigned i = 0; i < 1000; i++)
+sum += foo[i];
+ return sum;
+}
Index: lib/CodeGen/BackendUtil.cpp
===
--- lib/CodeGen/BackendUtil.cpp
+++ lib/CodeGen/BackendUtil.cpp
@@ -562,7 +562,7 @@
addCoroutinePassesToExtensionPoints(PMBuilder);
if (LangOpts.Sanitize.has(SanitizerKind::LocalBounds)) {
-PMBuilder.addExtension(PassManagerBuilder::EP_ScalarOptimizerLate,
+PMBuilder.addExtension(PassManagerBuilder::EP_EarlyAsPossible,
addBoundsCheckingPass);
PMBuilder.addExtension(PassManagerBuilder::EP_EnabledOnOptLevel0,
addBoundsCheckingPass);
Index: test/CodeGen/bounds-checking-opt.c
===
--- /dev/null
+++ test/CodeGen/bounds-checking-opt.c
@@ -0,0 +1,20 @@
+// RUN: %clang -S -fsanitize=local-bounds -emit-llvm -target x86_64-- %s -o - | FileCheck -check-prefix=O0 %s
+// RUN: %clang -S -fsanitize=local-bounds -emit-llvm -target x86_64-- -O2 %s -o - | FileCheck -check-prefix=O2 %s
+// RUN: %clang -S -fsanitize=local-bounds -emit-llvm -target aarch64-- %s -o - | FileCheck -check-prefix=O0 %s
+// RUN: %clang -S -fsanitize=local-bounds -emit-llvm -target aarch64-- -O2 %s -o - | FileCheck -check-prefix=O2 %s
+
+extern void fill(int *arr);
+
+// CHECK-LABEL: @f
+int f(int x) {
+ // O0: call {{.*}} @llvm.trap
+ // O2-NOT: call {{.*}} @llvm.trap
+ int foo[1000];
+ fill(foo);
+ int sum = 0;
+ #pragma clang loop vectorize(disable)
+ #pragma clang loop unroll(disable)
+ for (unsigned i = 0; i < 1000; i++)
+sum += foo[i];
+ return sum;
+}
Index: lib/CodeGen/BackendUtil.cpp
===
--- lib/CodeGen/BackendUtil.cpp
+++ lib/CodeGen/BackendUtil.cpp
@@ -562,7 +562,7 @@
addCoroutinePassesToExtensionPoints(PMBuilder);
if (LangOpts.Sanitize.has(SanitizerKind::LocalBounds)) {
-PMBuilder.addExtension(PassManagerBuilder::EP_ScalarOptimizerLate,
+PMBuilder.addExtension(PassManagerBuilder::EP_EarlyAsPossible,
addBoundsCheckingPass);
PMBuilder.addExtension(PassManagerBuilder::EP_EnabledOnOptLevel0,
addBoundsCheckingPass);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D49492: Run bounds checking sanitizer earlier to make it easier to optimize away its checks.
jgalenson added a comment. In https://reviews.llvm.org/D49492#1167064, @efriedma wrote: > Are you sure this will actually do what you want, in general? I suspect it > will end up missing bounds checks in some cases because it's running it too > early (before mem2reg/inlining/etc). No, I'm not sure; that's one reason I'm asking for comments. But I don't see any specific problems. For example, I don't see why inlining would matter; the checks should still be added, just before inlining instead of after (which of course affects the inlining heuristic, but that's another matter). I don't understand mem2reg as well, though. Do you have specific examples you think might fail? I was thinking about this in terms of other sanitizers I know, specifically the integer overflow sanitizer. That adds overflow checks in Clang, which is before all of these LLVM passes. So my thought was that moving bounds checks to be inserted earlier brings it closer to how the integer overflow sanitizer works. Repository: rC Clang https://reviews.llvm.org/D49492 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D49492: Run bounds checking sanitizer earlier to make it easier to optimize away its checks.
jgalenson added a comment. Good call; I had figured that running it earlier might just impede other optimizations, but I forgot that it would also hide size information. I thought this was the easiest approach compared to changing the pass pipeline or adding extra checks in here. But I hadn't realized I could use SCEV that easily. From trying it quickly, I think this can remove the check in this testcase (and even some others this couldn't remove). I'll work on making a patch that does that instead. Thanks for the quick feedback and suggestions! Repository: rC Clang https://reviews.llvm.org/D49492 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
