[llvm-exegesis] Add explicit support for setting DF in X86

[boomanaiden154]

While llvm-exegesis has explicit support for setting EFLAGS which contains DF, it can be nice sometimes to explicitly set DF, especially given that it is modeled as a separate register within LLVM. This patch adds the ability to do that by lowering setting the value to 0 or 1 to cld and std respectively.

[llvmbot]

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

Author: Aiden Grossman (boomanaiden154)

Changes

While llvm-exegesis has explicit support for setting EFLAGS which contains DF, it can be nice sometimes to explicitly set DF, especially given that it is modeled as a separate register within LLVM. This patch adds the ability to do that by lowering setting the value to 0 or 1 to cld and std respectively.

---

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

2 Files Affected:

• (modified) llvm/tools/llvm-exegesis/lib/X86/Target.cpp (+13)
• (modified) llvm/unittests/tools/llvm-exegesis/X86/TargetTest.cpp (+8)

diff --git a/llvm/tools/llvm-exegesis/lib/X86/Target.cpp b/llvm/tools/llvm-exegesis/lib/X86/Target.cpp
index 0a70321fab7818..3c3bff76fb6812 100644
--- a/llvm/tools/llvm-exegesis/lib/X86/Target.cpp
+++ b/llvm/tools/llvm-exegesis/lib/X86/Target.cpp
@@ -537,6 +537,8 @@ struct ConstantInliner {
   std::vector<MCInst> loadImplicitRegAndFinalize(unsigned Opcode,
                                                  unsigned Value);
 
+  std::vector<MCInst> loadDirectionFlagAndFinalize();
+
 private:
   ConstantInliner &add(const MCInst &Inst) {
     Instructions.push_back(Inst);
@@ -612,6 +614,15 @@ ConstantInliner::loadImplicitRegAndFinalize(unsigned Opcode, unsigned Value) {
   return std::move(Instructions);
 }
 
+std::vector<MCInst> ConstantInliner::loadDirectionFlagAndFinalize() {
+  if (Constant_.isZero())
+    add(MCInstBuilder(X86::CLD));
+  else if (Constant_.isOne())
+    add(MCInstBuilder(X86::STD));
+
+  return std::move(Instructions);
+}
+
 void ConstantInliner::initStack(unsigned Bytes) {
   assert(Constant_.getBitWidth() <= Bytes * 8 &&
          "Value does not have the correct size");
@@ -1089,6 +1100,8 @@ std::vector<MCInst> ExegesisX86Target::setRegTo(const MCSubtargetInfo &STI,
         0x1f80);
   if (Reg == X86::FPCW)
     return CI.loadImplicitRegAndFinalize(X86::FLDCW16m, 0x37f);
+  if (Reg == X86::DF)
+    return CI.loadDirectionFlagAndFinalize();
   return {}; // Not yet implemented.
 }
 
diff --git a/llvm/unittests/tools/llvm-exegesis/X86/TargetTest.cpp b/llvm/unittests/tools/llvm-exegesis/X86/TargetTest.cpp
index 921d7d7975f6ae..3dff50c44798d7 100644
--- a/llvm/unittests/tools/llvm-exegesis/X86/TargetTest.cpp
+++ b/llvm/unittests/tools/llvm-exegesis/X86/TargetTest.cpp
@@ -585,6 +585,14 @@ TEST_F(X86Core2TargetTest, SetRegToFP1_4Bits) {
                           OpcodeIs(X86::LD_Fp80m), IsStackDeallocate(10)));
 }
 
+TEST_F(X86Core2TargetTest, SetRegToDf1) {
+  EXPECT_THAT(setRegTo(X86::DF, APInt(1, 1)), ElementsAre(OpcodeIs(X86::STD)));
+}
+
+TEST_F(X86Core2TargetTest, SetRegToDf0) {
+  EXPECT_THAT(setRegTo(X86::DF, APInt(1, 0)), ElementsAre(OpcodeIs(X86::CLD)));
+}
+
 TEST_F(X86Core2Avx512TargetTest, FillMemoryOperands_ADD64rm) {
   const Instruction &I = getInstr(X86::ADD64rm);
   InstructionTemplate IT(&I);

[RKSimon]

Are you intending to add REP test coverage?

[boomanaiden154]

Are you intending to add REP test coverage?

Not currently. This is mostly to make it easier to run snippets while inferring which registers they use in a downstream project given LLVM models DF explicitly.

[RKSimon]

The ABI requires that DF is cleared before a function returns - are we guaranteeing that?

[boomanaiden154]

The ABI requires that DF is cleared before a function returns - are we guaranteeing that?

We are not. We're also not restoring eflags at all, which would have the same effect, so it seems like a separate issue that needs to be fixed.

It doesn't seem like the prologepilog pass ensures the relevant parts of rflags are set appropriately, at least for us. I'll look into that in a bit.

This also only matters for the in-process execution mode. When we call into the snippet in subprocess mode, it's no return (the MCJITed code makes an exit syscall). We should probably be disabling prolog/epilog insertion in that mode...

Given that, I'm not sure we should block on that. I've filed #116127 for tracking purpose. I'll try and put some other patches up to fix the other issues.

[legrosbuffle]

It doesn't seem like the prologepilog pass ensures the relevant parts of rflags are set appropriately, at least for us.

Looking at the list of callee-saved regs in X86CallingConv.td we have:

def CSR_SysV64_RegCall_NoSSE : CalleeSavedRegs<(add RBX, RBP,
                                               (sequence "R%u", 12, 15))>;
def CSR_SysV64_RegCall       : CalleeSavedRegs<(add CSR_SysV64_RegCall_NoSSE,               
                                               (sequence "XMM%u", 8, 15))>;

EFLAGS is not in there, I'm wondering why ?

[legrosbuffle]

EFLAGS is not in there, I'm wondering why ?

Hm wait, actually only DF is required to be cleared, sorry for the noise.

"The direction flag DF in the %rFLAGS register must be clear (set to “forward”
direction) on function entry and return. Other user flags have no specified role in
the standard calling sequence and are not preserved across calls"

[boomanaiden154]

Hm wait, actually only DF is required to be cleared, sorry for the noise.

Right. df isn't in there either, but I think that might just be an oversight.

Either way, something that I plan on addressing in a future patch to keep things focused, given this already occurs when setting eflags ourselves.

[RKSimon]

LGTM - cheers