[llvm-branch-commits] [llvm] [BOLT][BAT] Add entries for deleted basic blocks (PR #91906)
https://github.com/aaupov edited https://github.com/llvm/llvm-project/pull/91906 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] [BOLT][BAT] Add entries for deleted basic blocks (PR #91906)
https://github.com/aaupov updated
https://github.com/llvm/llvm-project/pull/91906
>From 972047a832f4fe67170f08f4f75bd3c0bb614021 Mon Sep 17 00:00:00 2001
From: Amir Ayupov
Date: Sun, 12 May 2024 17:37:48 -0700
Subject: [PATCH 1/2] fix test
Created using spr 1.3.4
---
bolt/test/X86/bb-with-two-tail-calls.s | 11 ++-
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/bolt/test/X86/bb-with-two-tail-calls.s
b/bolt/test/X86/bb-with-two-tail-calls.s
index c0b01e1894a49..8ea719262d155 100644
--- a/bolt/test/X86/bb-with-two-tail-calls.s
+++ b/bolt/test/X86/bb-with-two-tail-calls.s
@@ -10,8 +10,9 @@
# RUN: %clang %cflags %t.o -o %t.exe -Wl,-q -nostdlib
# RUN: llvm-bolt %t.exe -o %t.out --data %t.fdata --lite=0 --dyno-stats \
# RUN:--print-sctc --print-only=_start -enable-bat 2>&1 | FileCheck %s
-# RUN: llvm-objdump --syms %t.out | FileCheck %s --check-prefix=CHECK-NM
-# RUN: llvm-bat-dump %t.out --dump-all | FileCheck %s --check-prefix=CHECK-BAT
+# RUN: llvm-objdump --syms %t.out > %t.log
+# RUN: llvm-bat-dump %t.out --dump-all >> %t.log
+# RUN: FileCheck %s --input-file %t.log --check-prefix=CHECK-BAT
# CHECK-NOT: Assertion `BranchInfo.size() == 2 && "could only be called for
blocks with 2 successors"' failed.
# Two tail calls in the same basic block after SCTC:
@@ -19,9 +20,9 @@
# CHECK-NEXT:{{.*}}: jmp {{.*}} # TAILCALL # Offset: 12
# Confirm that a deleted basic block is emitted at function end offset (0xe)
-# CHECK-NM: 0060 g .text 000e _start
-# CHECK-BAT: Function Address: 0x60, hash: 0xf8bf620b266cdc1b
-# CHECK-BAT: 0xe -> 0xc hash: 0x823623240f000c
+# CHECK-BAT: [[#%x,ADDR:]] g .text [[#%x,SIZE:]] _start
+# CHECK-BAT: Function Address: 0x[[#%x,ADDR]]
+# CHECK-BAT: 0x[[#%x,SIZE]]
# CHECK-BAT: NumBlocks: 5
.globl _start
>From 8cf2305cbd12d6667b586fdb8125bc1310fce0be Mon Sep 17 00:00:00 2001
From: Amir Ayupov
Date: Sun, 12 May 2024 17:46:54 -0700
Subject: [PATCH 2/2] updated tests
Created using spr 1.3.4
---
bolt/test/X86/bolt-address-translation-yaml.test | 2 +-
bolt/test/X86/bolt-address-translation.test | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/bolt/test/X86/bolt-address-translation-yaml.test
b/bolt/test/X86/bolt-address-translation-yaml.test
index e21513b7dfe59..e28ca5c6c3488 100644
--- a/bolt/test/X86/bolt-address-translation-yaml.test
+++ b/bolt/test/X86/bolt-address-translation-yaml.test
@@ -40,7 +40,7 @@ RUN: | FileCheck --check-prefix CHECK-BOLT-YAML %s
WRITE-BAT-CHECK: BOLT-INFO: Wrote 5 BAT maps
WRITE-BAT-CHECK: BOLT-INFO: Wrote 4 function and 22 basic block hashes
-WRITE-BAT-CHECK: BOLT-INFO: BAT section size (bytes): 384
+WRITE-BAT-CHECK: BOLT-INFO: BAT section size (bytes): 404
READ-BAT-CHECK-NOT: BOLT-ERROR: unable to save profile in YAML format for
input file processed by BOLT
READ-BAT-CHECK: BOLT-INFO: Parsed 5 BAT entries
diff --git a/bolt/test/X86/bolt-address-translation.test
b/bolt/test/X86/bolt-address-translation.test
index e6b21c14077b4..dfdd1eea32333 100644
--- a/bolt/test/X86/bolt-address-translation.test
+++ b/bolt/test/X86/bolt-address-translation.test
@@ -37,7 +37,7 @@
# CHECK: BOLT: 3 out of 7 functions were overwritten.
# CHECK: BOLT-INFO: Wrote 6 BAT maps
# CHECK: BOLT-INFO: Wrote 3 function and 58 basic block hashes
-# CHECK: BOLT-INFO: BAT section size (bytes): 928
+# CHECK: BOLT-INFO: BAT section size (bytes): 940
#
# usqrt mappings (hot part). We match against any key (left side containing
# the bolted binary offsets) because BOLT may change where it puts instructions
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] [BOLT][BAT] Add entries for deleted basic blocks (PR #91906)
https://github.com/aaupov updated
https://github.com/llvm/llvm-project/pull/91906
>From 972047a832f4fe67170f08f4f75bd3c0bb614021 Mon Sep 17 00:00:00 2001
From: Amir Ayupov
Date: Sun, 12 May 2024 17:37:48 -0700
Subject: [PATCH] fix test
Created using spr 1.3.4
---
bolt/test/X86/bb-with-two-tail-calls.s | 11 ++-
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/bolt/test/X86/bb-with-two-tail-calls.s
b/bolt/test/X86/bb-with-two-tail-calls.s
index c0b01e1894a49..8ea719262d155 100644
--- a/bolt/test/X86/bb-with-two-tail-calls.s
+++ b/bolt/test/X86/bb-with-two-tail-calls.s
@@ -10,8 +10,9 @@
# RUN: %clang %cflags %t.o -o %t.exe -Wl,-q -nostdlib
# RUN: llvm-bolt %t.exe -o %t.out --data %t.fdata --lite=0 --dyno-stats \
# RUN:--print-sctc --print-only=_start -enable-bat 2>&1 | FileCheck %s
-# RUN: llvm-objdump --syms %t.out | FileCheck %s --check-prefix=CHECK-NM
-# RUN: llvm-bat-dump %t.out --dump-all | FileCheck %s --check-prefix=CHECK-BAT
+# RUN: llvm-objdump --syms %t.out > %t.log
+# RUN: llvm-bat-dump %t.out --dump-all >> %t.log
+# RUN: FileCheck %s --input-file %t.log --check-prefix=CHECK-BAT
# CHECK-NOT: Assertion `BranchInfo.size() == 2 && "could only be called for
blocks with 2 successors"' failed.
# Two tail calls in the same basic block after SCTC:
@@ -19,9 +20,9 @@
# CHECK-NEXT:{{.*}}: jmp {{.*}} # TAILCALL # Offset: 12
# Confirm that a deleted basic block is emitted at function end offset (0xe)
-# CHECK-NM: 0060 g .text 000e _start
-# CHECK-BAT: Function Address: 0x60, hash: 0xf8bf620b266cdc1b
-# CHECK-BAT: 0xe -> 0xc hash: 0x823623240f000c
+# CHECK-BAT: [[#%x,ADDR:]] g .text [[#%x,SIZE:]] _start
+# CHECK-BAT: Function Address: 0x[[#%x,ADDR]]
+# CHECK-BAT: 0x[[#%x,SIZE]]
# CHECK-BAT: NumBlocks: 5
.globl _start
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [BOLT][BAT] Add entries for deleted basic blocks (PR #91906)
https://github.com/aaupov updated https://github.com/llvm/llvm-project/pull/91906 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [BOLT][BAT] Add entries for deleted basic blocks (PR #91906)
https://github.com/aaupov updated https://github.com/llvm/llvm-project/pull/91906 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] [BOLT] Map branch source address to the containing basic block in BAT YAML (PR #91273)
https://github.com/aaupov updated https://github.com/llvm/llvm-project/pull/91273 >From 2a848b1ac92203f7821d79ffe7cf0d04846542f3 Mon Sep 17 00:00:00 2001 From: Amir Ayupov Date: Mon, 6 May 2024 16:00:01 -0700 Subject: [PATCH] add blarge_new_bat_branchentry.preagg.txt Created using spr 1.3.4 --- bolt/test/X86/Inputs/blarge_new_bat_branchentry.preagg.txt | 1 + 1 file changed, 1 insertion(+) create mode 100644 bolt/test/X86/Inputs/blarge_new_bat_branchentry.preagg.txt diff --git a/bolt/test/X86/Inputs/blarge_new_bat_branchentry.preagg.txt b/bolt/test/X86/Inputs/blarge_new_bat_branchentry.preagg.txt new file mode 100644 index 0..546da92f94dba --- /dev/null +++ b/bolt/test/X86/Inputs/blarge_new_bat_branchentry.preagg.txt @@ -0,0 +1 @@ +B 80010c 800194 1 0 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] [BOLT] Map branch source address to the containing basic block in BAT YAML (PR #91273)
https://github.com/aaupov updated https://github.com/llvm/llvm-project/pull/91273 >From 2a848b1ac92203f7821d79ffe7cf0d04846542f3 Mon Sep 17 00:00:00 2001 From: Amir Ayupov Date: Mon, 6 May 2024 16:00:01 -0700 Subject: [PATCH] add blarge_new_bat_branchentry.preagg.txt Created using spr 1.3.4 --- bolt/test/X86/Inputs/blarge_new_bat_branchentry.preagg.txt | 1 + 1 file changed, 1 insertion(+) create mode 100644 bolt/test/X86/Inputs/blarge_new_bat_branchentry.preagg.txt diff --git a/bolt/test/X86/Inputs/blarge_new_bat_branchentry.preagg.txt b/bolt/test/X86/Inputs/blarge_new_bat_branchentry.preagg.txt new file mode 100644 index 0..546da92f94dba --- /dev/null +++ b/bolt/test/X86/Inputs/blarge_new_bat_branchentry.preagg.txt @@ -0,0 +1 @@ +B 80010c 800194 1 0 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [BOLT][BAT] Add entries for deleted basic blocks (PR #91906)
llvmbot wrote:
@llvm/pr-subscribers-bolt
Author: Amir Ayupov (aaupov)
Changes
Deleted basic blocks are required for correct mapping of branches
modified by SCTC.
Test Plan: updated bb-with-two-tail-calls.s
---
Full diff: https://github.com/llvm/llvm-project/pull/91906.diff
4 Files Affected:
- (modified) bolt/docs/BAT.md (+5)
- (modified) bolt/include/bolt/Profile/BoltAddressTranslation.h (+1)
- (modified) bolt/lib/Profile/BoltAddressTranslation.cpp (+13)
- (modified) bolt/test/X86/bb-with-two-tail-calls.s (+9)
``diff
diff --git a/bolt/docs/BAT.md b/bolt/docs/BAT.md
index 7ffb5d7c00816..817ad288aa34b 100644
--- a/bolt/docs/BAT.md
+++ b/bolt/docs/BAT.md
@@ -106,9 +106,14 @@ equals output offset.
`BRANCHENTRY` bit denotes whether a given offset pair is a control flow source
(branch or call instruction). If not set, it signifies a control flow target
(basic block offset).
+
`InputAddr` is omitted for equal offsets in input and output function. In this
case, `BRANCHENTRY` bits are encoded separately in a `BranchEntries` bitvector.
+Deleted basic blocks are emitted as having `OutputOffset` equal to the size of
+the function. They don't affect address translation and only participate in
+input basic block mapping.
+
### Secondary Entry Points table
The table is emitted for hot fragments only. It contains `NumSecEntryPoints`
offsets denoting secondary entry points, delta encoded, implicitly starting at
zero.
diff --git a/bolt/include/bolt/Profile/BoltAddressTranslation.h
b/bolt/include/bolt/Profile/BoltAddressTranslation.h
index 68b993ee363cc..16fe0442f10a5 100644
--- a/bolt/include/bolt/Profile/BoltAddressTranslation.h
+++ b/bolt/include/bolt/Profile/BoltAddressTranslation.h
@@ -217,6 +217,7 @@ class BoltAddressTranslation {
auto begin() const { return Map.begin(); }
auto end() const { return Map.end(); }
auto upper_bound(uint32_t Offset) const { return Map.upper_bound(Offset); }
+auto size() const { return Map.size(); }
};
/// Map function output address to its hash and basic blocks hash map.
diff --git a/bolt/lib/Profile/BoltAddressTranslation.cpp
b/bolt/lib/Profile/BoltAddressTranslation.cpp
index 7cfb9c132c2c6..724b348c7845e 100644
--- a/bolt/lib/Profile/BoltAddressTranslation.cpp
+++ b/bolt/lib/Profile/BoltAddressTranslation.cpp
@@ -108,6 +108,19 @@ void BoltAddressTranslation::write(const BinaryContext
, raw_ostream ) {
for (const BinaryBasicBlock *const BB :
Function.getLayout().getMainFragment())
writeEntriesForBB(Map, *BB, InputAddress, OutputAddress);
+// Add entries for deleted blocks. They are still required for correct BB
+// mapping of branches modified by SCTC. By convention, they would have the
+// end of the function as output address.
+const BBHashMapTy = getBBHashMap(InputAddress);
+if (BBHashMap.size() != Function.size()) {
+ const uint64_t EndOffset = Function.getOutputSize();
+ std::unordered_set MappedInputOffsets;
+ for (const BinaryBasicBlock : Function)
+MappedInputOffsets.emplace(BB.getInputOffset());
+ for (const auto &[InputOffset, _] : BBHashMap)
+if (!llvm::is_contained(MappedInputOffsets, InputOffset))
+ Map[EndOffset] = InputOffset << 1;
+}
Maps.emplace(Function.getOutputAddress(), std::move(Map));
ReverseMap.emplace(OutputAddress, InputAddress);
diff --git a/bolt/test/X86/bb-with-two-tail-calls.s
b/bolt/test/X86/bb-with-two-tail-calls.s
index bb2b0cd4cc23a..c0b01e1894a49 100644
--- a/bolt/test/X86/bb-with-two-tail-calls.s
+++ b/bolt/test/X86/bb-with-two-tail-calls.s
@@ -10,11 +10,20 @@
# RUN: %clang %cflags %t.o -o %t.exe -Wl,-q -nostdlib
# RUN: llvm-bolt %t.exe -o %t.out --data %t.fdata --lite=0 --dyno-stats \
# RUN:--print-sctc --print-only=_start -enable-bat 2>&1 | FileCheck %s
+# RUN: llvm-objdump --syms %t.out | FileCheck %s --check-prefix=CHECK-NM
+# RUN: llvm-bat-dump %t.out --dump-all | FileCheck %s --check-prefix=CHECK-BAT
+
# CHECK-NOT: Assertion `BranchInfo.size() == 2 && "could only be called for
blocks with 2 successors"' failed.
# Two tail calls in the same basic block after SCTC:
# CHECK: {{.*}}: ja {{.*}} # TAILCALL # Offset: 7 #
CTCTakenCount: 4
# CHECK-NEXT:{{.*}}: jmp {{.*}} # TAILCALL # Offset: 12
+# Confirm that a deleted basic block is emitted at function end offset (0xe)
+# CHECK-NM: 0060 g .text 000e _start
+# CHECK-BAT: Function Address: 0x60, hash: 0xf8bf620b266cdc1b
+# CHECK-BAT: 0xe -> 0xc hash: 0x823623240f000c
+# CHECK-BAT: NumBlocks: 5
+
.globl _start
_start:
je x
``
https://github.com/llvm/llvm-project/pull/91906
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [BOLT][BAT] Add entries for deleted basic blocks (PR #91906)
https://github.com/aaupov created https://github.com/llvm/llvm-project/pull/91906 Deleted basic blocks are required for correct mapping of branches modified by SCTC. Test Plan: updated bb-with-two-tail-calls.s ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] [BOLT] Map branch source address to the containing basic block in BAT YAML (PR #91273)
https://github.com/maksfb approved this pull request. Please see the nit for the error message (caps and new line). Otherwise LGTM. https://github.com/llvm/llvm-project/pull/91273 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] [BOLT] Map branch source address to the containing basic block in BAT YAML (PR #91273)
@@ -2378,21 +2379,27 @@ std::error_code
DataAggregator::writeBATYAML(BinaryContext ,
return CSI;
};
+ // Lookup containing basic block offset and index
+ auto getBlock = [](uint32_t Offset) {
+auto BlockIt = BlockMap.upper_bound(Offset);
+if (LLVM_UNLIKELY(BlockIt == BlockMap.begin())) {
+ errs() << "BOLT-ERROR: Invalid BAT section";
maksfb wrote:
```suggestion
errs() << "BOLT-ERROR: invalid BAT section\n";
```
https://github.com/llvm/llvm-project/pull/91273
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] [BOLT] Map branch source address to the containing basic block in BAT YAML (PR #91273)
https://github.com/maksfb edited https://github.com/llvm/llvm-project/pull/91273 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [BOLT] Preserve Offset annotation in fixDoubleJumps (PR #91898)
https://github.com/aaupov closed https://github.com/llvm/llvm-project/pull/91898 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [BOLT] Preserve Offset annotation in fixDoubleJumps (PR #91898)
https://github.com/aaupov updated https://github.com/llvm/llvm-project/pull/91898 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [BOLT] Preserve Offset annotation in fixDoubleJumps (PR #91898)
https://github.com/aaupov updated https://github.com/llvm/llvm-project/pull/91898 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [BOLT] Preserve Offset annotation in fixDoubleJumps (PR #91898)
https://github.com/dcci approved this pull request. LG https://github.com/llvm/llvm-project/pull/91898 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] [BOLT] Map branch source address to the containing basic block in BAT YAML (PR #91273)
https://github.com/aaupov updated https://github.com/llvm/llvm-project/pull/91273 >From 2a848b1ac92203f7821d79ffe7cf0d04846542f3 Mon Sep 17 00:00:00 2001 From: Amir Ayupov Date: Mon, 6 May 2024 16:00:01 -0700 Subject: [PATCH] add blarge_new_bat_branchentry.preagg.txt Created using spr 1.3.4 --- bolt/test/X86/Inputs/blarge_new_bat_branchentry.preagg.txt | 1 + 1 file changed, 1 insertion(+) create mode 100644 bolt/test/X86/Inputs/blarge_new_bat_branchentry.preagg.txt diff --git a/bolt/test/X86/Inputs/blarge_new_bat_branchentry.preagg.txt b/bolt/test/X86/Inputs/blarge_new_bat_branchentry.preagg.txt new file mode 100644 index 0..546da92f94dba --- /dev/null +++ b/bolt/test/X86/Inputs/blarge_new_bat_branchentry.preagg.txt @@ -0,0 +1 @@ +B 80010c 800194 1 0 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] [BOLT] Map branch source address to the containing basic block in BAT YAML (PR #91273)
https://github.com/aaupov updated https://github.com/llvm/llvm-project/pull/91273 >From 2a848b1ac92203f7821d79ffe7cf0d04846542f3 Mon Sep 17 00:00:00 2001 From: Amir Ayupov Date: Mon, 6 May 2024 16:00:01 -0700 Subject: [PATCH] add blarge_new_bat_branchentry.preagg.txt Created using spr 1.3.4 --- bolt/test/X86/Inputs/blarge_new_bat_branchentry.preagg.txt | 1 + 1 file changed, 1 insertion(+) create mode 100644 bolt/test/X86/Inputs/blarge_new_bat_branchentry.preagg.txt diff --git a/bolt/test/X86/Inputs/blarge_new_bat_branchentry.preagg.txt b/bolt/test/X86/Inputs/blarge_new_bat_branchentry.preagg.txt new file mode 100644 index 0..546da92f94dba --- /dev/null +++ b/bolt/test/X86/Inputs/blarge_new_bat_branchentry.preagg.txt @@ -0,0 +1 @@ +B 80010c 800194 1 0 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [BOLT] Preserve Offset annotation in fixDoubleJumps (PR #91898)
llvmbot wrote:
@llvm/pr-subscribers-bolt
Author: Amir Ayupov (aaupov)
Changes
Offset annotation was missed when optimizing an unconditional branch to
a tail call.
Test Plan: update bb-with-two-tail-calls.s
---
Full diff: https://github.com/llvm/llvm-project/pull/91898.diff
2 Files Affected:
- (modified) bolt/lib/Passes/BinaryPasses.cpp (+3)
- (modified) bolt/test/X86/bb-with-two-tail-calls.s (+3-3)
``diff
diff --git a/bolt/lib/Passes/BinaryPasses.cpp b/bolt/lib/Passes/BinaryPasses.cpp
index df6dbcddeed56..867f977cebca7 100644
--- a/bolt/lib/Passes/BinaryPasses.cpp
+++ b/bolt/lib/Passes/BinaryPasses.cpp
@@ -715,6 +715,9 @@ static uint64_t fixDoubleJumps(BinaryFunction ,
bool MarkInvalid) {
Pred->removeSuccessor();
Pred->eraseInstruction(Pred->findInstruction(Branch));
Pred->addTailCallInstruction(SuccSym);
+ MCInst *TailCall = Pred->getLastNonPseudoInstr();
+ assert(TailCall);
+ MIB->setOffset(*TailCall, BB.getOffset());
} else {
return false;
}
diff --git a/bolt/test/X86/bb-with-two-tail-calls.s
b/bolt/test/X86/bb-with-two-tail-calls.s
index caad7b3d735f5..bb2b0cd4cc23a 100644
--- a/bolt/test/X86/bb-with-two-tail-calls.s
+++ b/bolt/test/X86/bb-with-two-tail-calls.s
@@ -9,11 +9,11 @@
# RUN: llvm-strip --strip-unneeded %t.o
# RUN: %clang %cflags %t.o -o %t.exe -Wl,-q -nostdlib
# RUN: llvm-bolt %t.exe -o %t.out --data %t.fdata --lite=0 --dyno-stats \
-# RUN:--print-sctc --print-only=_start 2>&1 | FileCheck %s
+# RUN:--print-sctc --print-only=_start -enable-bat 2>&1 | FileCheck %s
# CHECK-NOT: Assertion `BranchInfo.size() == 2 && "could only be called for
blocks with 2 successors"' failed.
# Two tail calls in the same basic block after SCTC:
-# CHECK: {{.*}}: ja {{.*}} # TAILCALL # CTCTakenCount: {{.*}}
-# CHECK-NEXT:{{.*}}: jmp {{.*}} # TAILCALL
+# CHECK: {{.*}}: ja {{.*}} # TAILCALL # Offset: 7 #
CTCTakenCount: 4
+# CHECK-NEXT:{{.*}}: jmp {{.*}} # TAILCALL # Offset: 12
.globl _start
_start:
``
https://github.com/llvm/llvm-project/pull/91898
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [BOLT] Preserve Offset annotation in fixDoubleJumps (PR #91898)
https://github.com/aaupov created https://github.com/llvm/llvm-project/pull/91898 Offset annotation was missed when optimizing an unconditional branch to a tail call. Test Plan: update bb-with-two-tail-calls.s ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] [BOLT] Map branch source address to the containing basic block in BAT YAML (PR #91273)
@@ -2378,21 +2378,24 @@ std::error_code
DataAggregator::writeBATYAML(BinaryContext ,
return CSI;
};
+ // Lookup containing basic block offset and index
+ auto getBlock = [](uint32_t Offset) {
+auto BlockIt = BlockMap.upper_bound(Offset);
+assert(BlockIt != BlockMap.begin());
+--BlockIt;
+return std::pair(BlockIt->first, BlockIt->second.getBBIndex());
+ };
+
for (const auto &[FromOffset, SuccKV] : Branches.IntraIndex) {
-if (!BlockMap.isInputBlock(FromOffset))
- continue;
-const unsigned Index = BlockMap.getBBIndex(FromOffset);
+const auto &[_, Index] = getBlock(FromOffset);
maksfb wrote:
Even though we generate BAT in BOLT, when we view the invocation of BOLT on a
binary with embedded BAT, such input should be considered an external and
potentially malformed data. In this case, assertions will not provide adequate
enough protection since we can build BOLT without them.
https://github.com/llvm/llvm-project/pull/91273
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [clang] release/18.x: [Clang][Sema] Revise the transformation of CTAD parameters of nested class templates (#91628) (PR #91890)
https://github.com/llvmbot updated
https://github.com/llvm/llvm-project/pull/91890
>From 7dcca34943097af1863f5a8718a41c888bedb682 Mon Sep 17 00:00:00 2001
From: Younan Zhang
Date: Fri, 10 May 2024 20:47:15 +0800
Subject: [PATCH] [Clang][Sema] Revise the transformation of CTAD parameters of
nested class templates (#91628)
This fixes a regression introduced by bee78b88f.
When we form a deduction guide for a constructor, basically, we do the
following work:
- Collect template parameters from the constructor's surrounding class
template, if present.
- Collect template parameters from the constructor.
- Splice these template parameters together into a new template
parameter list.
- Turn all the references (e.g. the function parameter list) to the
invented parameter list by applying a `TreeTransform` to the function
type.
In the previous fix, we handled cases of nested class templates by
substituting the "outer" template parameters (i.e. those not declared at
the surrounding class template or the constructor) with the
instantiating template arguments. The approach per se makes sense, but
there was a flaw in the following case:
```cpp
template struct X {
template struct Y {
template Y(T) {}
};
template Y(T) -> Y;
};
X::Y y(42);
```
While we're transforming the parameters for `Y(T)`, we first attempt to
transform all references to `V` and `T`; then, we handle the references
to outer parameters `U` and `Us` using the template arguments from
`X` by transforming the same `ParamDecl`. However, the first step
results in the reference `T` being `` because the
invented `T` is the last of the parameter list of the deduction guide,
and what we're substituting with is a corresponding parameter pack
(which is `Us`, though empty). Hence we're messing up the substitution.
I think we can resolve it by reversing the substitution order, which
means handling outer template parameters first and then the inner
parameters.
There's no release note because this is a regression in 18, and I hope
we can catch up with the last release.
Fixes https://github.com/llvm/llvm-project/issues/88142
(cherry picked from commit 8c852ab57932a5cd954cb0d050c3d2ab486428df)
---
clang/lib/Sema/SemaTemplate.cpp | 25 ++-
.../nested-implicit-deduction-guides.cpp | 14 +++
2 files changed, 33 insertions(+), 6 deletions(-)
diff --git a/clang/lib/Sema/SemaTemplate.cpp b/clang/lib/Sema/SemaTemplate.cpp
index b619f5d729e86..a12a64939c464 100644
--- a/clang/lib/Sema/SemaTemplate.cpp
+++ b/clang/lib/Sema/SemaTemplate.cpp
@@ -2404,9 +2404,6 @@ struct ConvertConstructorToDeductionGuideTransform {
Args.addOuterRetainedLevel();
}
-if (NestedPattern)
- Args.addOuterRetainedLevels(NestedPattern->getTemplateDepth());
-
FunctionProtoTypeLoc FPTL = CD->getTypeSourceInfo()->getTypeLoc()
.getAsAdjusted();
assert(FPTL && "no prototype for constructor declaration");
@@ -2526,11 +2523,27 @@ struct ConvertConstructorToDeductionGuideTransform {
//-- The types of the function parameters are those of the constructor.
for (auto *OldParam : TL.getParams()) {
- ParmVarDecl *NewParam =
- transformFunctionTypeParam(OldParam, Args, MaterializedTypedefs);
- if (NestedPattern && NewParam)
+ ParmVarDecl *NewParam = OldParam;
+ // Given
+ // template struct C {
+ // template struct D {
+ // template D(U, V);
+ // };
+ // };
+ // First, transform all the references to template parameters that are
+ // defined outside of the surrounding class template. That is T in the
+ // above example.
+ if (NestedPattern) {
NewParam = transformFunctionTypeParam(NewParam, OuterInstantiationArgs,
MaterializedTypedefs);
+if (!NewParam)
+ return QualType();
+ }
+ // Then, transform all the references to template parameters that are
+ // defined at the class template and the constructor. In this example,
+ // they're U and V, respectively.
+ NewParam =
+ transformFunctionTypeParam(NewParam, Args, MaterializedTypedefs);
if (!NewParam)
return QualType();
ParamTypes.push_back(NewParam->getType());
diff --git a/clang/test/SemaTemplate/nested-implicit-deduction-guides.cpp
b/clang/test/SemaTemplate/nested-implicit-deduction-guides.cpp
index 38b6706595a11..f289dc0452868 100644
--- a/clang/test/SemaTemplate/nested-implicit-deduction-guides.cpp
+++ b/clang/test/SemaTemplate/nested-implicit-deduction-guides.cpp
@@ -84,3 +84,17 @@ nested_init_list::concept_fail nil_invalid{1, ""};
// expected-note@#INIT_LIST_INNER_INVALID {{candidate template ignored:
substitution failure [with F = const char *]: constraints not satisfied for
class template 'concept_fail' [with F = const char *]}}
// expected-note@#INIT_LIST_INNER_INVALID {{candidate
[llvm-branch-commits] [clang] release/18.x: [Clang][Sema] Revise the transformation of CTAD parameters of nested class templates (#91628) (PR #91890)
llvmbot wrote:
@llvm/pr-subscribers-clang
Author: None (llvmbot)
Changes
Backport 8c852ab57932
Requested by: @zyn0217
---
Full diff: https://github.com/llvm/llvm-project/pull/91890.diff
2 Files Affected:
- (modified) clang/lib/Sema/SemaTemplate.cpp (+19-6)
- (modified) clang/test/SemaTemplate/nested-implicit-deduction-guides.cpp (+14)
``diff
diff --git a/clang/lib/Sema/SemaTemplate.cpp b/clang/lib/Sema/SemaTemplate.cpp
index b619f5d729e86..a12a64939c464 100644
--- a/clang/lib/Sema/SemaTemplate.cpp
+++ b/clang/lib/Sema/SemaTemplate.cpp
@@ -2404,9 +2404,6 @@ struct ConvertConstructorToDeductionGuideTransform {
Args.addOuterRetainedLevel();
}
-if (NestedPattern)
- Args.addOuterRetainedLevels(NestedPattern->getTemplateDepth());
-
FunctionProtoTypeLoc FPTL = CD->getTypeSourceInfo()->getTypeLoc()
.getAsAdjusted();
assert(FPTL && "no prototype for constructor declaration");
@@ -2526,11 +2523,27 @@ struct ConvertConstructorToDeductionGuideTransform {
//-- The types of the function parameters are those of the constructor.
for (auto *OldParam : TL.getParams()) {
- ParmVarDecl *NewParam =
- transformFunctionTypeParam(OldParam, Args, MaterializedTypedefs);
- if (NestedPattern && NewParam)
+ ParmVarDecl *NewParam = OldParam;
+ // Given
+ // template struct C {
+ // template struct D {
+ // template D(U, V);
+ // };
+ // };
+ // First, transform all the references to template parameters that are
+ // defined outside of the surrounding class template. That is T in the
+ // above example.
+ if (NestedPattern) {
NewParam = transformFunctionTypeParam(NewParam, OuterInstantiationArgs,
MaterializedTypedefs);
+if (!NewParam)
+ return QualType();
+ }
+ // Then, transform all the references to template parameters that are
+ // defined at the class template and the constructor. In this example,
+ // they're U and V, respectively.
+ NewParam =
+ transformFunctionTypeParam(NewParam, Args, MaterializedTypedefs);
if (!NewParam)
return QualType();
ParamTypes.push_back(NewParam->getType());
diff --git a/clang/test/SemaTemplate/nested-implicit-deduction-guides.cpp
b/clang/test/SemaTemplate/nested-implicit-deduction-guides.cpp
index 38b6706595a11..f289dc0452868 100644
--- a/clang/test/SemaTemplate/nested-implicit-deduction-guides.cpp
+++ b/clang/test/SemaTemplate/nested-implicit-deduction-guides.cpp
@@ -84,3 +84,17 @@ nested_init_list::concept_fail nil_invalid{1, ""};
// expected-note@#INIT_LIST_INNER_INVALID {{candidate template ignored:
substitution failure [with F = const char *]: constraints not satisfied for
class template 'concept_fail' [with F = const char *]}}
// expected-note@#INIT_LIST_INNER_INVALID {{candidate function template not
viable: requires 1 argument, but 2 were provided}}
// expected-note@#INIT_LIST_INNER_INVALID {{candidate function template not
viable: requires 0 arguments, but 2 were provided}}
+
+namespace GH88142 {
+
+template struct X {
+ template struct Y {
+template Y(T) {}
+ };
+
+ template Y(T) -> Y;
+};
+
+X::Y y(42);
+
+} // namespace PR88142
``
https://github.com/llvm/llvm-project/pull/91890
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [clang] release/18.x: [Clang][Sema] Revise the transformation of CTAD parameters of nested class templates (#91628) (PR #91890)
llvmbot wrote: @zyn0217 What do you think about merging this PR to the release branch? https://github.com/llvm/llvm-project/pull/91890 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [clang] release/18.x: [Clang][Sema] Revise the transformation of CTAD parameters of nested class templates (#91628) (PR #91890)
https://github.com/llvmbot milestoned https://github.com/llvm/llvm-project/pull/91890 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [clang] release/18.x: [Clang][Sema] Revise the transformation of CTAD parameters of nested class templates (#91628) (PR #91890)
https://github.com/llvmbot created
https://github.com/llvm/llvm-project/pull/91890
Backport 8c852ab57932
Requested by: @zyn0217
>From e45b88f445fdbc90c0ba40f31ef264ba8a20baa9 Mon Sep 17 00:00:00 2001
From: Younan Zhang
Date: Fri, 10 May 2024 20:47:15 +0800
Subject: [PATCH] [Clang][Sema] Revise the transformation of CTAD parameters of
nested class templates (#91628)
This fixes a regression introduced by bee78b88f.
When we form a deduction guide for a constructor, basically, we do the
following work:
- Collect template parameters from the constructor's surrounding class
template, if present.
- Collect template parameters from the constructor.
- Splice these template parameters together into a new template
parameter list.
- Turn all the references (e.g. the function parameter list) to the
invented parameter list by applying a `TreeTransform` to the function
type.
In the previous fix, we handled cases of nested class templates by
substituting the "outer" template parameters (i.e. those not declared at
the surrounding class template or the constructor) with the
instantiating template arguments. The approach per se makes sense, but
there was a flaw in the following case:
```cpp
template struct X {
template struct Y {
template Y(T) {}
};
template Y(T) -> Y;
};
X::Y y(42);
```
While we're transforming the parameters for `Y(T)`, we first attempt to
transform all references to `V` and `T`; then, we handle the references
to outer parameters `U` and `Us` using the template arguments from
`X` by transforming the same `ParamDecl`. However, the first step
results in the reference `T` being `` because the
invented `T` is the last of the parameter list of the deduction guide,
and what we're substituting with is a corresponding parameter pack
(which is `Us`, though empty). Hence we're messing up the substitution.
I think we can resolve it by reversing the substitution order, which
means handling outer template parameters first and then the inner
parameters.
There's no release note because this is a regression in 18, and I hope
we can catch up with the last release.
Fixes https://github.com/llvm/llvm-project/issues/88142
(cherry picked from commit 8c852ab57932a5cd954cb0d050c3d2ab486428df)
---
clang/lib/Sema/SemaTemplate.cpp | 25 ++-
.../nested-implicit-deduction-guides.cpp | 14 +++
2 files changed, 33 insertions(+), 6 deletions(-)
diff --git a/clang/lib/Sema/SemaTemplate.cpp b/clang/lib/Sema/SemaTemplate.cpp
index b619f5d729e86..a12a64939c464 100644
--- a/clang/lib/Sema/SemaTemplate.cpp
+++ b/clang/lib/Sema/SemaTemplate.cpp
@@ -2404,9 +2404,6 @@ struct ConvertConstructorToDeductionGuideTransform {
Args.addOuterRetainedLevel();
}
-if (NestedPattern)
- Args.addOuterRetainedLevels(NestedPattern->getTemplateDepth());
-
FunctionProtoTypeLoc FPTL = CD->getTypeSourceInfo()->getTypeLoc()
.getAsAdjusted();
assert(FPTL && "no prototype for constructor declaration");
@@ -2526,11 +2523,27 @@ struct ConvertConstructorToDeductionGuideTransform {
//-- The types of the function parameters are those of the constructor.
for (auto *OldParam : TL.getParams()) {
- ParmVarDecl *NewParam =
- transformFunctionTypeParam(OldParam, Args, MaterializedTypedefs);
- if (NestedPattern && NewParam)
+ ParmVarDecl *NewParam = OldParam;
+ // Given
+ // template struct C {
+ // template struct D {
+ // template D(U, V);
+ // };
+ // };
+ // First, transform all the references to template parameters that are
+ // defined outside of the surrounding class template. That is T in the
+ // above example.
+ if (NestedPattern) {
NewParam = transformFunctionTypeParam(NewParam, OuterInstantiationArgs,
MaterializedTypedefs);
+if (!NewParam)
+ return QualType();
+ }
+ // Then, transform all the references to template parameters that are
+ // defined at the class template and the constructor. In this example,
+ // they're U and V, respectively.
+ NewParam =
+ transformFunctionTypeParam(NewParam, Args, MaterializedTypedefs);
if (!NewParam)
return QualType();
ParamTypes.push_back(NewParam->getType());
diff --git a/clang/test/SemaTemplate/nested-implicit-deduction-guides.cpp
b/clang/test/SemaTemplate/nested-implicit-deduction-guides.cpp
index 38b6706595a11..f289dc0452868 100644
--- a/clang/test/SemaTemplate/nested-implicit-deduction-guides.cpp
+++ b/clang/test/SemaTemplate/nested-implicit-deduction-guides.cpp
@@ -84,3 +84,17 @@ nested_init_list::concept_fail nil_invalid{1, ""};
// expected-note@#INIT_LIST_INNER_INVALID {{candidate template ignored:
substitution failure [with F = const char *]: constraints not satisfied for
class template 'concept_fail' [with F = const char *]}}
//
[llvm-branch-commits] [llvm] release/18.x: [PPCMergeStringPool] Avoid replacing constant with instruction (#88846) (PR #91557)
https://github.com/chenzheng1030 approved this pull request. LGTM https://github.com/llvm/llvm-project/pull/91557 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
