Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[clang][X86] X86::LAR X86::LSL add_implicate eflags #80993

Merged
merged 1 commit into from
Feb 15, 2024

Conversation

Qfrost911
Copy link
Contributor

@xia0ji233 and I found that X86::LAR and X86::lSR implicit use eflags register. However, it was not been defined in LLVM, which means we will get wrong alive-result if we use these two instructions.
T~ _81W6A}J}{AP{DF%E}KY

@llvmbot
Copy link
Collaborator

llvmbot commented Feb 7, 2024

@llvm/pr-subscribers-backend-x86

Author: Qfrost (Qfrost911)

Changes

@xia0ji233 and I found that X86::LAR and X86::lSR implicit use eflags register. However, it was not been defined in LLVM, which means we will get wrong alive-result if we use these two instructions.
T~ _81W6A}J}{AP{DF%E}KY


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

1 Files Affected:

  • (modified) llvm/lib/Target/X86/X86InstrSystem.td (+2)
diff --git a/llvm/lib/Target/X86/X86InstrSystem.td b/llvm/lib/Target/X86/X86InstrSystem.td
index a7899a2492b88..24a334d38f926 100644
--- a/llvm/lib/Target/X86/X86InstrSystem.td
+++ b/llvm/lib/Target/X86/X86InstrSystem.td
@@ -213,6 +213,7 @@ def MOV16sm : I<0x8E, MRMSrcMem, (outs SEGMENT_REG:$dst), (ins i16mem:$src),
 let SchedRW = [WriteSystem] in {
 def SWAPGS : I<0x01, MRM_F8, (outs), (ins), "swapgs", []>, TB;
 
+let Defs = [EFLAGS] in {
 let mayLoad = 1 in
 def LAR16rm : I<0x02, MRMSrcMem, (outs GR16:$dst), (ins i16mem:$src),
                 "lar{w}\t{$src, $dst|$dst, $src}", []>, TB,
@@ -253,6 +254,7 @@ def LSL64rm : RI<0x03, MRMSrcMem, (outs GR64:$dst), (ins i16mem:$src),
                  "lsl{q}\t{$src, $dst|$dst, $src}", []>, TB;
 def LSL64rr : RI<0x03, MRMSrcReg, (outs GR64:$dst), (ins GR16orGR32orGR64:$src),
                  "lsl{q}\t{$src, $dst|$dst, $src}", []>, TB;
+}
 
 def INVLPG : I<0x01, MRM7m, (outs), (ins i8mem:$addr), "invlpg\t$addr", []>, TB;
 

@Qfrost911 Qfrost911 changed the title [clang] X86::LAR X86::LSL add_implicate eflags [clang][X86] X86::LAR X86::LSL add_implicate eflags Feb 14, 2024
@Qfrost911
Copy link
Contributor Author

@pranavk @MaskRay @KanRobert Excuse me, I think this is a nice patch, but I don't know to whom reviewer should I turn.

Copy link
Collaborator

@topperc topperc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@Qfrost911 Qfrost911 merged commit bfe302c into llvm:main Feb 15, 2024
5 of 6 checks passed
Copy link
Contributor

@KanRobert KanRobert left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants