Insert symbols for prefetch targets read from basic blocks section profile. #168439

rlavaee · 2025-11-17T20:51:49Z

This is the first PR to enable the prefetch optimization via Propeller based on our RFC. It enables emitting special symbols prefixed with __llvm_prefetch_target to point to the prefetch targets as specified via directives in the profile. A prefetch target is uniquely identified by its function name, basic block ID, and the subblock index (used when the target is after a call instruction).

A new pass is added which sets a field in basic blocks which have prefetch targets. The next PR will add the prefetch insertion logic into the same pass.

github-actions · 2025-11-17T21:40:54Z

🐧 Linux x64 Test Results

186408 tests passed
4858 tests skipped

david-xl · 2025-11-17T21:20:12Z

llvm/include/llvm/CodeGen/BasicBlockSectionsProfileReader.h

+// the block if `SubblockIndex` is the last call in the block).
+struct SubblockID {
+  UniqueBBID BBID;
+  unsigned SubblockIndex;


Just the callsite index within the block?

Had originally named this callsiteIndex, but it will be a misnomer if there are not calls in the block (where we will use the index 0). What do u suggest?

we can define two 'pseudo calls' one at the beginning of a block and one at the end, so the first real call always get id == 1.

Besides we may want to differentiate between the code point before and after the call, so perhaps we can use negative sign for it. For instance t 2,-1 means the target is at block 2 before the first call, while t 2,1 means the target is in block 2 after the first call.

CallSiteIndex seems more readable to me. It is fine that when it is zero it means the start of the block.

Prefetch targets and instructions are only placed at the beginning of a subblock (either the beginning of the block , or after calls). So we don't need the negativity.

we can define two 'pseudo calls' one at the beginning of a block and one at the end, so the first real call always get id == 1.

No need for a pseudo call at the end, but with one at the beginning, we get subBlockIndex+1. I can change it back to int callsiteIndex and have callsiteIndex == -1 refer to the prefetch target being mapped to the beginning of the block. WDYT about this?

Actually, I went back and changed it to unsigned 1-based callsiteIndexes. I explain in comments that callsite index of zero means the start of the block.

david-xl · 2025-11-17T21:28:45Z

llvm/lib/CodeGen/InsertCodePrefetch.cpp

+  DenseMap<UniqueBBID, SmallVector<unsigned>> PrefetchTargetsByBBID;
+  for (const auto &Target : PrefetchTargets)
+    PrefetchTargetsByBBID[Target.BBID].push_back(Target.SubblockIndex);
+  for (auto &MBB : MF) {


Possible to iterate the BBIDs and use MF->getBlockNumbered(ID) method to get MBB?

We don't store a map from BBIDs to MBBs. So there is no way around once iterating over MBBs if that's what you mean (We can create the map here but it would still require an iteration).

david-xl · 2025-11-17T21:44:22Z

llvm/test/CodeGen/X86/basic-block-sections-code-prefetch.ll

+;;
+;; Specify the bb sections profile:
+; RUN: echo 'v1' > %t
+; RUN: echo 'f _Z3foob' >> %t


The format is different from what is described in RFC which uses , instead of @

Thanks. I changed it to use comma. Alternatively, we can use spaces which will make the parsing code simpler.

MatzeB · 2025-11-18T00:20:27Z

Is this usable for anyone outside of google? I'm not fluent in the propeller codebase, so possible I overlooked something, but I don't see any code dealing with instruction prefetching in the propeller code at https://github.com/google/llvm-propeller (main branch)...

rlavaee · 2025-11-18T22:30:35Z

Is this usable for anyone outside of google? I'm not fluent in the propeller codebase, so possible I overlooked something, but I don't see any code dealing with instruction prefetching in the propeller code at https://github.com/google/llvm-propeller (main branch)...

The feature was added just today to the main branch: google/llvm-propeller@f36e59d

We have also discussed how this pass can be reused in other frameworks in the RFC; TLDR is we'll get there when we get there. Currently, we rely on BB_ADDR_MAP to do precise mapping to subblocks.

tmsri

Partially reviewed.

tmsri · 2025-11-17T22:20:32Z

llvm/include/llvm/CodeGen/BasicBlockSectionsProfileReader.h

+// the block if `SubblockIndex` is the last call in the block).
+struct SubblockID {
+  UniqueBBID BBID;
+  unsigned SubblockIndex;


CallSiteIndex seems more readable to me. It is fine that when it is zero it means the start of the block.

tmsri · 2025-11-17T22:22:40Z

llvm/include/llvm/CodeGen/BasicBlockSectionsProfileReader.h

  // the edge a -> b (a is not cloned). The index of the path in this vector
  // determines the `UniqueBBID::CloneID` of the cloned blocks in that path.
  SmallVector<SmallVector<unsigned>> ClonePaths;
+  // Code prefetch targets, specified by the subblock ID of which beginning must


Suggest rephrasing this comment to something like: "Code Prefetch Target list, the beginning of each Subblock is the target".

tmsri · 2025-11-17T22:28:11Z

llvm/include/llvm/CodeGen/MachineBasicBlock.h

+  /// `i`th call (or the beginning of the block if `i==0`) to before the`i+1`th
+  /// callsite (or the end of the block). The prefetch target is always the
+  /// beginning of the subblock.
+  SmallVector<unsigned> PrefetchTargetSubblockIndexes;


define CallSiteIndex or SubblockIndex as unsigned type and use that instead of unsigned?

tmsri · 2025-11-19T23:17:04Z

llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp

+    };
+
    for (auto &MI : MBB) {
+      EmitPrefetchTargetSymbolIfNeeded();


I would have EmitPrefetchTargetIt take a parameter which is the unsigned callsiteindex. I would remove the updates to the PrefetchTargets from the lambda and do it within the loop and replace this line with:

if (PrefetchTargetIt != PrefetchTargets.end() && *PrefetchTargetIt == LastCallSiteIndex) { EmitPrefetchTargetSymbolIfNeeded(*PrefetchTargetIt); ++PrefetchTargetIt; }

tmsri · 2025-11-19T23:18:01Z

llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp

+        // version, in which case the prefetch targets should also be replaced.
+        OutStreamer->emitSymbolAttribute(
+            PrefetchTargetSymbol,
+            MF->getFunction().isWeakForLinker() ? MCSA_Weak : MCSA_Global);


What happens to internal linkage? Can you make this symbol a global, that would conflict.

Right. We are relying on funique as mentioned in the RFC. Should we add a check here to ensure funique has been used (it's possible that funique is not applied on some functions).

rlavaee requested review from WenleiHe, david-xl, spupyrev and tmsri November 17, 2025 20:52

david-xl reviewed Nov 17, 2025

View reviewed changes

WenleiHe requested review from MatzeB and wlei-llvm November 17, 2025 23:37

rlavaee force-pushed the prefetch-symbols branch from b676430 to b3202a1 Compare November 19, 2025 07:08

rlavaee added 19 commits November 19, 2025 21:04

feat(AsmPrinter): Add support for emitting prefetch target symbols

49c5f22

feat: Add prefetch-profile.txt for testing

b25adef

Everything else.

bbfb7ba

Add test.

3e6b04f

Fix everything

9967360

clang-format.

a08b65a

Fix the prefetch test.

d988a3c

Rename the test.

4008445

Remove unrelated changes.

e3b501f

Add some comments.

715f1b8

clang-format.

a1e1e00

Add comments and rename functions.

717e6fe

clang-format.

3605b0d

Add optimization remarks for when prefetch targets cannot be mapped.

6408bd7

Expand test to weak symbols.

a06cb9d

Change prefetch directive format to use , instead of @

ceefc56

Fix the error.

639efd7

Remove optimization remarks.

cc4e333

Refine and polish.

6d8bdb1

rlavaee added 4 commits November 19, 2025 21:05

clang-format.

d93a5ec

Change to using unsigned values for CallsiteIndex

7cb4f6b

clang-format.

9fdf7d0

Fix AsmPrinter.

0c17e45

rlavaee force-pushed the prefetch-symbols branch from 85649e7 to 0c17e45 Compare November 19, 2025 21:05

use -O0

500b536

tmsri reviewed Nov 19, 2025

View reviewed changes

rlavaee mentioned this pull request Nov 19, 2025

X86: Add prefetch insertion based on Propeller profile #166324

Open

tmsri reviewed Nov 19, 2025

View reviewed changes

Insert symbols for prefetch targets read from basic blocks section profile. #168439

Are you sure you want to change the base?

Insert symbols for prefetch targets read from basic blocks section profile. #168439

Uh oh!

Conversation

rlavaee commented Nov 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Nov 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🐧 Linux x64 Test Results

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

MatzeB commented Nov 18, 2025

Uh oh!

rlavaee commented Nov 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tmsri left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

rlavaee commented Nov 17, 2025 •

edited

Loading

github-actions bot commented Nov 17, 2025 •

edited

Loading

rlavaee commented Nov 18, 2025 •

edited

Loading