[X86] Fix typo: QWORD alignment is greater than or equal to 8, not greater than 8

[AZero13]

Align(8) is QWORD aligned, but this was checking to see if alignment was greater than that, when it should have been checking for being greater than OR EQUAL to Align(8).

This bug was introduced in 6a6af30d433d7 during the transition to the Align type.

[llvmbot]

@llvm/pr-subscribers-backend-x86

Author: AtariDreams (AtariDreams)

Changes

This probably inhibited a lot of optimizations. EmitTargetCodeForMemcpy does not have this problem, luckily.

---

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

1 Files Affected:

• (modified) llvm/lib/Target/X86/X86SelectionDAGInfo.cpp (+1-1)

diff --git a/llvm/lib/Target/X86/X86SelectionDAGInfo.cpp b/llvm/lib/Target/X86/X86SelectionDAGInfo.cpp
index 7c630a2b0da080..bc5cb5934454ce 100644
--- a/llvm/lib/Target/X86/X86SelectionDAGInfo.cpp
+++ b/llvm/lib/Target/X86/X86SelectionDAGInfo.cpp
@@ -86,7 +86,7 @@ SDValue X86SelectionDAGInfo::EmitTargetCodeForMemset(
       ValReg = X86::EAX;
       Val = (Val << 8)  | Val;
       Val = (Val << 16) | Val;
-      if (Subtarget.is64Bit() && Alignment > Align(8)) { // QWORD aligned
+      if (Subtarget.is64Bit() && Alignment > Align(4)) { // QWORD aligned
         AVT = MVT::i64;
         ValReg = X86::RAX;
         Val = (Val << 32) | Val;

[topperc]

But no tests are affected?

[AZero13]

But no tests are affected?

Align(8) is QWORD aligned, but this was checking to see f alignment was higher than that, which it is not.

Another PR I am working on for optsize depends on this, but because it is an oversight or typo that affects the compiler, I figure I would make this into a separate PR.

[topperc]

But no tests are affected?

Align(8) is QWORD aligned, but this was checking to see f alignment was higher than that, which it is not.

Another PR I am working on for optsize depends on this, but because it is an oversight or typo that affects the compiler, I figure I would make this into a separate PR.

Your original description said "This probably inhibited a lot of optimizations.". That should be pretty easy to prove so why no test updates?

[AZero13]

But no tests are affected?

Align(8) is QWORD aligned, but this was checking to see f alignment was higher than that, which it is not.
Another PR I am working on for optsize depends on this, but because it is an oversight or typo that affects the compiler, I figure I would make this into a separate PR.

Your original description said "This probably inhibited a lot of optimizations.". That should be pretty easy to prove so why no test updates?

Because I am writing those tests right now.

[AZero13]

But no tests are affected?

Align(8) is QWORD aligned, but this was checking to see f alignment was higher than that, which it is not.

Another PR I am working on for optsize depends on this, but because it is an oversight or typo that affects the compiler, I figure I would make this into a separate PR.

Your original description said "This probably inhibited a lot of optimizations.". That should be pretty easy to prove so why no test updates?

Done!

[AZero13]

@phoebewang ping?

[AZero13]

I don't have commit access by the way.

[AZero13]

/cherry-pick 8389b3b

[llvmbot]

/cherry-pick 8389b3b

Error: Command failed due to missing milestone.