Skip to content

Conversation

arichardson
Copy link
Member

@arichardson arichardson commented Dec 7, 2024

We are indexing into an array here, so we can use
CreateConstInBoundsGEP2_64 instead of the manual arithmetic.

Created using spr 1.3.6-beta.1
@llvmbot
Copy link
Member

llvmbot commented Dec 7, 2024

@llvm/pr-subscribers-llvm-transforms

@llvm/pr-subscribers-compiler-rt-sanitizer

Author: Alexander Richardson (arichardson)

Changes

We are indexing into an array here, so we can use
CreateConstInBoundsGEP1_64 instead of the manual arithmetic.


Full diff: https://github.com/llvm/llvm-project/pull/119097.diff

3 Files Affected:

  • (modified) llvm/lib/Transforms/Instrumentation/SanitizerCoverage.cpp (+2-4)
  • (modified) llvm/test/Instrumentation/SanitizerCoverage/cmp-tracing-api-x86_32.ll (+2-1)
  • (modified) llvm/test/Instrumentation/SanitizerCoverage/missing_dbg.ll (+38-11)
diff --git a/llvm/lib/Transforms/Instrumentation/SanitizerCoverage.cpp b/llvm/lib/Transforms/Instrumentation/SanitizerCoverage.cpp
index 22acf59c78a385..2d6816f0ef5b8f 100644
--- a/llvm/lib/Transforms/Instrumentation/SanitizerCoverage.cpp
+++ b/llvm/lib/Transforms/Instrumentation/SanitizerCoverage.cpp
@@ -1045,10 +1045,8 @@ void ModuleSanitizerCoverage::InjectCoverageAtBlock(Function &F, BasicBlock &BB,
         ->setCannotMerge(); // gets the PC using GET_CALLER_PC.
   }
   if (Options.TracePCGuard) {
-    auto GuardPtr = IRB.CreateIntToPtr(
-        IRB.CreateAdd(IRB.CreatePointerCast(FunctionGuardArray, IntptrTy),
-                      ConstantInt::get(IntptrTy, Idx * 4)),
-        PtrTy);
+    auto GuardPtr = IRB.CreateConstInBoundsGEP1_64(
+        FunctionGuardArray->getValueType(), FunctionGuardArray, Idx);
     if (Options.GatedCallbacks) {
       Instruction *I = &*IP;
       auto GateBranch = CreateGateBranch(F, FunctionGateCmp, I);
diff --git a/llvm/test/Instrumentation/SanitizerCoverage/cmp-tracing-api-x86_32.ll b/llvm/test/Instrumentation/SanitizerCoverage/cmp-tracing-api-x86_32.ll
index 6ade3e32bd2791..e57ff078a5274f 100644
--- a/llvm/test/Instrumentation/SanitizerCoverage/cmp-tracing-api-x86_32.ll
+++ b/llvm/test/Instrumentation/SanitizerCoverage/cmp-tracing-api-x86_32.ll
@@ -7,13 +7,14 @@ target triple = "i386-unknown-linux-gnu"
 define i32 @foo() #0 {
 ; CHECK-LABEL: define i32 @foo() comdat {
 ; CHECK-NEXT:  entry:
-; CHECK-NEXT:    call void @__sanitizer_cov_trace_pc_guard(ptr inttoptr (i32 ptrtoint (ptr @__sancov_gen_ to i32) to ptr)) #[[ATTR1:[0-9]+]]
+; CHECK-NEXT:    call void @__sanitizer_cov_trace_pc_guard(ptr @__sancov_gen_) #[[ATTR1:[0-9]+]]
 ; CHECK-NEXT:    ret i32 0
 ;
 entry:
   ret i32 0
 }
 
+; UTC_ARGS: --disable
 ; CHECK-DAG: declare void @__sanitizer_cov_trace_pc_indir(i32)
 ; CHECK-DAG: declare void @__sanitizer_cov_trace_cmp1(i8 zeroext, i8 zeroext)
 ; CHECK-DAG: declare void @__sanitizer_cov_trace_cmp2(i16 zeroext, i16 zeroext)
diff --git a/llvm/test/Instrumentation/SanitizerCoverage/missing_dbg.ll b/llvm/test/Instrumentation/SanitizerCoverage/missing_dbg.ll
index 21c6fcdb3a84b0..360ef6891c337e 100644
--- a/llvm/test/Instrumentation/SanitizerCoverage/missing_dbg.ll
+++ b/llvm/test/Instrumentation/SanitizerCoverage/missing_dbg.ll
@@ -1,8 +1,23 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 5
 ; RUN: opt < %s -passes='module(sancov-module)' -sanitizer-coverage-level=2 -S | FileCheck %s
 
 target datalayout = "e-p:64:64:64-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:64:64-f32:32:32-f64:64:64-v64:64:64-v128:128:128-a0:0:64-s0:64:64-f80:128:128-n8:16:32:64-S128"
 
 define i32 @with_dbg(ptr %a, ptr %b) !dbg !3 {
+; CHECK-LABEL: define i32 @with_dbg(
+; CHECK-SAME: ptr [[A:%.*]], ptr [[B:%.*]]) comdat !dbg [[DBG3:![0-9]+]] {
+; CHECK-NEXT:  [[ENTRY:.*:]]
+; CHECK-NEXT:    call void @__sanitizer_cov_trace_pc_guard(ptr @__sancov_gen_) #[[ATTR1:[0-9]+]], !dbg [[DBG6:![0-9]+]]
+; CHECK-NEXT:    [[TMP1:%.*]] = load i32, ptr [[A]], align 4
+; CHECK-NEXT:    [[CMP:%.*]] = icmp eq i32 [[TMP1]], 42
+; CHECK-NEXT:    br i1 [[CMP]], label %[[BB0:.*]], label %[[BB1:.*]]
+; CHECK:       [[BB0]]:
+; CHECK-NEXT:    call void @__sanitizer_cov_trace_pc_guard(ptr getelementptr inbounds ([2 x i32], ptr @__sancov_gen_, i64 1)) #[[ATTR1]], !dbg [[DBG7:![0-9]+]]
+; CHECK-NEXT:    store i32 [[TMP1]], ptr [[B]], align 4
+; CHECK-NEXT:    br label %[[BB1]]
+; CHECK:       [[BB1]]:
+; CHECK-NEXT:    ret i32 [[TMP1]]
+;
 entry:
   %tmp1 = load i32, ptr %a, align 4
   %cmp = icmp eq i32 %tmp1, 42
@@ -13,12 +28,22 @@ entry:
 1:
   ret i32 %tmp1
 }
-; CHECK-LABEL: @with_dbg
-; CHECK-NEXT:  entry:
-; CHECK:       call void @__sanitizer_cov_trace_pc_guard(ptr @__sancov_gen_) #1, !dbg [[DBG1:![0-9]+]]
-; CHECK:       call void @__sanitizer_cov_trace_pc_guard(ptr inttoptr (i64 add (i64 ptrtoint (ptr @__sancov_gen_ to i64), i64 4) to ptr)) #1, !dbg [[DBG2:![0-9]+]]
 
 define i32 @without_dbg(ptr %a, ptr %b) {
+; CHECK-LABEL: define i32 @without_dbg(
+; CHECK-SAME: ptr [[A:%.*]], ptr [[B:%.*]]) comdat {
+; CHECK-NEXT:  [[ENTRY:.*:]]
+; CHECK-NEXT:    call void @__sanitizer_cov_trace_pc_guard(ptr @__sancov_gen_.1) #[[ATTR1]]
+; CHECK-NEXT:    [[TMP1:%.*]] = load i32, ptr [[A]], align 4
+; CHECK-NEXT:    [[CMP:%.*]] = icmp eq i32 [[TMP1]], 42
+; CHECK-NEXT:    br i1 [[CMP]], label %[[BB0:.*]], label %[[BB1:.*]]
+; CHECK:       [[BB0]]:
+; CHECK-NEXT:    call void @__sanitizer_cov_trace_pc_guard(ptr getelementptr inbounds ([2 x i32], ptr @__sancov_gen_.1, i64 1)) #[[ATTR1]]
+; CHECK-NEXT:    store i32 [[TMP1]], ptr [[B]], align 4
+; CHECK-NEXT:    br label %[[BB1]]
+; CHECK:       [[BB1]]:
+; CHECK-NEXT:    ret i32 [[TMP1]]
+;
 entry:
   %tmp1 = load i32, ptr %a, align 4
   %cmp = icmp eq i32 %tmp1, 42
@@ -29,10 +54,6 @@ entry:
 1:
   ret i32 %tmp1
 }
-; CHECK-LABEL: @without_dbg
-; CHECK-NEXT:  entry:
-; CHECK:       call void @__sanitizer_cov_trace_pc_guard(ptr @__sancov_gen_.1) #1
-; CHECK:       call void @__sanitizer_cov_trace_pc_guard(ptr inttoptr (i64 add (i64 ptrtoint (ptr @__sancov_gen_.1 to i64), i64 4) to ptr)) #1
 
 !llvm.dbg.cu = !{!0}
 !llvm.module.flags = !{!2}
@@ -45,6 +66,12 @@ entry:
 !5 = !{}
 !6 = !DILocation(line: 192, scope: !3)
 !7 = !DILocation(line: 0, scope: !3)
-
-; CHECK:       [[DBG1]] = !DILocation(line: 192, scope: !3)
-; CHECK:       [[DBG2]] = !DILocation(line: 0, scope: !3)
+;.
+; CHECK: [[META0:![0-9]+]] = distinct !DICompileUnit(language: DW_LANG_C89, file: [[META1:![0-9]+]], isOptimized: true, runtimeVersion: 0, emissionKind: LineTablesOnly, splitDebugInlining: false, nameTableKind: None)
+; CHECK: [[META1]] = !DIFile(filename: "foo.c", directory: "")
+; CHECK: [[DBG3]] = distinct !DISubprogram(name: "foo", scope: [[META1]], file: [[META1]], line: 190, type: [[META4:![0-9]+]], scopeLine: 192, flags: DIFlagPrototyped | DIFlagAllCallsDescribed, spFlags: DISPFlagLocalToUnit | DISPFlagDefinition | DISPFlagOptimized, unit: [[META0]])
+; CHECK: [[META4]] = !DISubroutineType(types: [[META5:![0-9]+]])
+; CHECK: [[META5]] = !{}
+; CHECK: [[DBG6]] = !DILocation(line: 192, scope: [[DBG3]])
+; CHECK: [[DBG7]] = !DILocation(line: 0, scope: [[DBG3]])
+;.

vitalybuka added a commit that referenced this pull request Dec 8, 2024
Created using spr 1.3.4
@vitalybuka vitalybuka self-requested a review December 8, 2024 05:19
@vitalybuka
Copy link
Collaborator

There is a few failures in check-compiler-rt

Copy link
Collaborator

@vitalybuka vitalybuka left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

More tests need update

Created using spr 1.3.6-beta.1
@vitalybuka vitalybuka removed the request for review from ramosian-glider December 9, 2024 05:50
@vitalybuka
Copy link
Collaborator

Added @ramosian-glider accidentally

@arichardson
Copy link
Member Author

Thanks for review! Will merge tomorrow morning

Created using spr 1.3.6-beta.1
@arichardson arichardson merged commit 4646cab into main Dec 9, 2024
2 of 4 checks passed
@arichardson arichardson deleted the users/arichardson/spr/sanitizercoverage-avoid-unnecessary-inttoptr branch December 9, 2024 17:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants