-
Notifications
You must be signed in to change notification settings - Fork 10.8k
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
[X86] Remove obsolete tablegen rules for near data in small static code model #84523
Conversation
…de model These should be already handled by other code. Removing the kernel code model rules right above it cause bss_pagealigned.ll to fail by using a movabsq to get the address of a global, haven't figured out where that code is yet.
@llvm/pr-subscribers-backend-x86 Author: Arthur Eubanks (aeubanks) ChangesThese should be already handled by other code. Removing the kernel code model rules right above it cause bss_pagealigned.ll to fail by using a movabsq to get the address of a global, haven't figured out where that code is yet. Full diff: https://github.com/llvm/llvm-project/pull/84523.diff 1 Files Affected:
diff --git a/llvm/lib/Target/X86/X86InstrCompiler.td b/llvm/lib/Target/X86/X86InstrCompiler.td
index f393f86e64aadd..ba8168042e0e98 100644
--- a/llvm/lib/Target/X86/X86InstrCompiler.td
+++ b/llvm/lib/Target/X86/X86InstrCompiler.td
@@ -1237,28 +1237,6 @@ def : Pat<(i64 (X86Wrapper mcsym:$dst)),
def : Pat<(i64 (X86Wrapper tblockaddress:$dst)),
(MOV64ri32 tblockaddress:$dst)>, Requires<[KernelCode]>;
-// If we have small model and -static mode, it is safe to store global addresses
-// directly as immediates. FIXME: This is really a hack, the 'imm' predicate
-// for MOV64mi32 should handle this sort of thing.
-def : Pat<(store (i64 (X86Wrapper tconstpool:$src)), addr:$dst),
- (MOV64mi32 addr:$dst, tconstpool:$src)>,
- Requires<[NearData, IsNotPIC]>;
-def : Pat<(store (i64 (X86Wrapper tjumptable:$src)), addr:$dst),
- (MOV64mi32 addr:$dst, tjumptable:$src)>,
- Requires<[NearData, IsNotPIC]>;
-def : Pat<(store (i64 (X86Wrapper tglobaladdr:$src)), addr:$dst),
- (MOV64mi32 addr:$dst, tglobaladdr:$src)>,
- Requires<[NearData, IsNotPIC]>;
-def : Pat<(store (i64 (X86Wrapper texternalsym:$src)), addr:$dst),
- (MOV64mi32 addr:$dst, texternalsym:$src)>,
- Requires<[NearData, IsNotPIC]>;
-def : Pat<(store (i64 (X86Wrapper mcsym:$src)), addr:$dst),
- (MOV64mi32 addr:$dst, mcsym:$src)>,
- Requires<[NearData, IsNotPIC]>;
-def : Pat<(store (i64 (X86Wrapper tblockaddress:$src)), addr:$dst),
- (MOV64mi32 addr:$dst, tblockaddress:$src)>,
- Requires<[NearData, IsNotPIC]>;
-
def : Pat<(i32 (X86RecoverFrameAlloc mcsym:$dst)), (MOV32ri mcsym:$dst)>;
def : Pat<(i64 (X86RecoverFrameAlloc mcsym:$dst)), (MOV64ri mcsym:$dst)>;
|
ping |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we have test coverage for this?
From reviewing past PRs, I would say that yes, we have adequate test coverage for x86 code models, small static, PIC, medium, etc, etc, so if the tests pass, I'm pretty confident these rules are not necessary. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM - cheers
These should be already handled by other code.
Removing the kernel code model rules right above it cause bss_pagealigned.ll to fail by using a movabsq to get the address of a global, haven't figured out where that code is yet.