[Exegesis] Implemented strategy for load operation #113458

AnastasiyaChernikova · 2024-10-23T13:40:07Z

This fix helps to map operand memory to destination registers. If instruction is load, we can self-alias it in case when instruction overrides whole address register. For that we use provided scratch memory.

llvmbot · 2024-10-23T13:40:46Z

@llvm/pr-subscribers-tools-llvm-exegesis

Author: None (AnastasiyaChernikova)

Changes

This fix helps to map operand memory to destination registers. If instruction is load, we can self-alias it in case when instruction overrides whole address register. For that we use provided scratch memory.

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

1 Files Affected:

(modified) llvm/tools/llvm-exegesis/lib/SerialSnippetGenerator.cpp (+45-1)

diff --git a/llvm/tools/llvm-exegesis/lib/SerialSnippetGenerator.cpp b/llvm/tools/llvm-exegesis/lib/SerialSnippetGenerator.cpp
index 7100b51bbb7298..ac4881f298618d 100644
--- a/llvm/tools/llvm-exegesis/lib/SerialSnippetGenerator.cpp
+++ b/llvm/tools/llvm-exegesis/lib/SerialSnippetGenerator.cpp
@@ -113,7 +113,51 @@ static void appendCodeTemplates(const LLVMState &State,
   }
   case ExecutionMode::SERIAL_VIA_MEMORY_INSTR: {
     // Select back-to-back memory instruction.
-    // TODO: Implement me.
+
+    auto &I = Variant.getInstr();
+    if (I.Description.mayLoad()) {
+      // If instruction is load, we can self-alias it in case when instruction
+      // overrides whole address register. For that we use provided scratch
+      // memory.
+
+      // TODO: now it is not checked if load writes the whole register.
+
+      auto DefOpIt = find_if(I.Operands, [](Operand const &op) {
+        return op.isDef() && op.isReg();
+      });
+
+      if (DefOpIt == I.Operands.end())
+        return;
+
+      const Operand &DefOp = *DefOpIt;
+      auto &ET = State.getExegesisTarget();
+      auto ScratchMemoryRegister = ET.getScratchMemoryRegister(
+          State.getTargetMachine().getTargetTriple());
+      auto &RegClass =
+          State.getTargetMachine().getMCRegisterInfo()->getRegClass(
+              DefOp.getExplicitOperandInfo().RegClass);
+
+      // Register classes of def operand and memory operand must be the same
+      // to perform aliasing.
+      if (!RegClass.contains(ScratchMemoryRegister))
+        return;
+
+      ET.fillMemoryOperands(Variant, ScratchMemoryRegister, 0);
+      Variant.getValueFor(DefOp) = MCOperand::createReg(ScratchMemoryRegister);
+
+      CodeTemplate CT;
+      CT.Execution = ExecutionModeBit;
+      if (CT.ScratchSpacePointerInReg == 0)
+        CT.ScratchSpacePointerInReg = ScratchMemoryRegister;
+
+      CT.Info = std::string(ExecutionClassDescription);
+      CT.Instructions.push_back(std::move(Variant));
+      CT.PreinitScratchMemory.emplace_back(ScratchMemoryRegister,
+                                           /* Offset */ 0);
+      CodeTemplates.push_back(std::move(CT));
+    }
+
+    // TODO: implement more cases
     return;
   }
   case ExecutionMode::SERIAL_VIA_EXPLICIT_REGS: {

AnastasiyaChernikova · 2024-10-28T13:24:34Z

@legrosbuffle @mshockwave @topperc @boomanaiden154
Please take a look

mshockwave

Do you have any test? Or does this depend on #89047

AnastasiyaChernikova · 2024-11-05T15:45:22Z

Yes, this is part of #89047, which was asked to be discussed in this comment (#89047 (comment)). This only affects boot support for riscv, it doesn't change anything on other platforms (I looked at the --opcode-index=-1 entries in the add code for all supported architectures). Since the commit with riscv support hasn't been pushed yet, I can't add tests here.

mshockwave · 2024-11-05T19:00:26Z

Yes, this is part of #89047, which was asked to be discussed in this comment (#89047 (comment)). This only affects boot support for riscv, it doesn't change anything on other platforms (I looked at the --opcode-index=-1 entries in the add code for all supported architectures). Since the commit with riscv support hasn't been pushed yet, I can't add tests here.

Is it possible to write test with other platforms, like ARM? If not, could you add a line in the PR description that this PR is stacked on top of #89047 ?

mshockwave · 2024-11-05T18:54:01Z

llvm/tools/llvm-exegesis/lib/SerialSnippetGenerator.cpp

+      auto DefOpIt = find_if(I.Operands, [](Operand const &op) {
+        return op.isDef() && op.isReg();
+      });


Suggested change

auto DefOpIt = find_if(I.Operands, [](Operand const &op) {

return op.isDef() && op.isReg();

});

auto DefOpIt = find_if(I.Operands, [](Operand const &Op) {

return Op.isDef() && Op.isReg();

});

Uppercase

mshockwave · 2024-11-05T18:57:18Z

llvm/tools/llvm-exegesis/lib/SerialSnippetGenerator.cpp

+
+      CodeTemplate CT;
+      CT.Execution = ExecutionModeBit;
+      if (CT.ScratchSpacePointerInReg == 0)


You just created CT a few lines back, is it possible that ScratchSpacePointerInReg is not zero at this point?

mshockwave · 2024-11-05T18:57:50Z

llvm/tools/llvm-exegesis/lib/SerialSnippetGenerator.cpp

+      auto &ET = State.getExegesisTarget();
+      auto ScratchMemoryRegister = ET.getScratchMemoryRegister(
+          State.getTargetMachine().getTargetTriple());
+      auto &RegClass =


please spell out the auto here

mshockwave · 2024-11-05T18:59:07Z

llvm/tools/llvm-exegesis/lib/CodeTemplate.h

@@ -132,6 +132,9 @@ struct CodeTemplate {
  // If the template uses the provided scratch memory, the register in which
  // the pointer to this memory is passed in to the function.
  unsigned ScratchSpacePointerInReg = 0;
+  // Require to pre-store value of a given register (fisrt)
+  // to scratch memory with given offset (second)
+  SmallVector<std::pair<unsigned, unsigned>, 2> PreinitScratchMemory;


this field is populated, but where is it used?

This fix helps to map operand memory to destination registers. If instruction is load, we can self-alias it in case when instruction overrides whole address register. For that we use provided scratch memory.

AnastasiyaChernikova · 2025-01-01T22:15:11Z

@mshockwave @legrosbuffle @topperc @boomanaiden154 I answered all the comments, please take a look

AnastasiyaChernikova · 2025-01-13T13:03:52Z

@mshockwave @legrosbuffle @topperc @boomanaiden154 I look forward to further review

AnastasiyaChernikova · 2025-01-29T10:10:32Z

@mshockwave @legrosbuffle @topperc @boomanaiden154 Please take a look

dybv-sc · 2025-02-04T10:36:54Z

@mshockwave, do you have any objections for this?

dybv-sc · 2025-03-03T12:48:23Z

It's been a month and there was no objections and this patch is LGTM, so I would say that it is good to merge. I'll wait till the end of this week though, so original reviews may take a look once more @mshockwave @legrosbuffle @topperc @boomanaiden154

mshockwave

Sorry for being late. ~~I only have one comment on the test file.~~ LGTM now

llvm/test/tools/llvm-exegesis/RISCV/latency-by-load.s

boomanaiden154

One nit, otherwise seems reasonable enough to me.

boomanaiden154 · 2025-03-03T17:24:55Z

llvm/tools/llvm-exegesis/lib/SerialSnippetGenerator.cpp

+      const ExegesisTarget &ET = State.getExegesisTarget();
+      unsigned ScratchMemoryRegister = ET.getScratchMemoryRegister(
+          State.getTargetMachine().getTargetTriple());
+      const llvm::MCRegisterClass &RegClass =


Nit: We can drop the llvm:: here.

This fix helps to map operand memory to destination registers. If instruction is load, we can self-alias it in case when instruction overrides whole address register. For that we use provided scratch memory.

llvmbot added the tools:llvm-exegesis label Oct 23, 2024

AnastasiyaChernikova mentioned this pull request Oct 23, 2024

[Exegesis][RISCV] Add RISCV support for llvm-exegesis #89047

Merged

boomanaiden154 requested review from legrosbuffle, mshockwave, topperc and boomanaiden154 October 24, 2024 20:38

AnastasiyaChernikova force-pushed the load branch from 016057e to 52cad46 Compare October 28, 2024 10:56

mshockwave reviewed Nov 4, 2024

View reviewed changes

mshockwave reviewed Nov 5, 2024

View reviewed changes

AnastasiyaChernikova added 2 commits December 24, 2024 14:14

[Exegesis] Implemented strategy for load operation

4acd123

This fix helps to map operand memory to destination registers. If instruction is load, we can self-alias it in case when instruction overrides whole address register. For that we use provided scratch memory.

Addressed comments and add test

3508c76

AnastasiyaChernikova force-pushed the load branch from 52cad46 to 3508c76 Compare December 24, 2024 13:54

Fix test

f76ffe4

AnastasiyaChernikova requested a review from mshockwave December 24, 2024 16:25

AnastasiyaChernikova mentioned this pull request Feb 3, 2025

Request Commit Access For AnastasiyaChernikova #125489

Closed

mshockwave reviewed Mar 3, 2025

View reviewed changes

llvm/test/tools/llvm-exegesis/RISCV/latency-by-load.s Show resolved Hide resolved

mshockwave approved these changes Mar 3, 2025

View reviewed changes

boomanaiden154 approved these changes Mar 3, 2025

View reviewed changes

dybv-sc merged commit 0fcbf14 into llvm:main Mar 4, 2025
8 checks passed

[Exegesis] Implemented strategy for load operation #113458

[Exegesis] Implemented strategy for load operation #113458

Uh oh!

Conversation

AnastasiyaChernikova commented Oct 23, 2024

Uh oh!

llvmbot commented Oct 23, 2024

Uh oh!

AnastasiyaChernikova commented Oct 28, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mshockwave left a comment

Choose a reason for hiding this comment

Uh oh!

AnastasiyaChernikova commented Nov 5, 2024

Uh oh!

mshockwave commented Nov 5, 2024

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!

AnastasiyaChernikova commented Jan 1, 2025

Uh oh!

AnastasiyaChernikova commented Jan 13, 2025

Uh oh!

AnastasiyaChernikova commented Jan 29, 2025

Uh oh!

dybv-sc commented Feb 4, 2025

Uh oh!

dybv-sc commented Mar 3, 2025

Uh oh!

mshockwave left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

boomanaiden154 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

AnastasiyaChernikova commented Oct 28, 2024 •

edited

Loading

mshockwave left a comment •

edited

Loading