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

[X86] Resolve FIXME: WinCOFFObjectWriter::RecordRelocation can emit PC-relative calls #83877

Closed
wants to merge 1 commit into from

Conversation

AtariDreams
Copy link
Contributor

This has been true for a while now, but this check has not been removed.

@llvmbot
Copy link
Collaborator

llvmbot commented Mar 4, 2024

@llvm/pr-subscribers-backend-x86

Author: AtariDreams (AtariDreams)

Changes

This has been true for a while now, but this check has not been removed.


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

1 Files Affected:

  • (modified) llvm/lib/Target/X86/X86Subtarget.cpp (+1-4)
diff --git a/llvm/lib/Target/X86/X86Subtarget.cpp b/llvm/lib/Target/X86/X86Subtarget.cpp
index 07f535685e8f97..74d9b1bd327ea1 100644
--- a/llvm/lib/Target/X86/X86Subtarget.cpp
+++ b/llvm/lib/Target/X86/X86Subtarget.cpp
@@ -238,10 +238,7 @@ X86Subtarget::classifyGlobalFunctionReference(const GlobalValue *GV,
 
 /// Return true if the subtarget allows calls to immediate address.
 bool X86Subtarget::isLegalToCallImmediateAddr() const {
-  // FIXME: I386 PE/COFF supports PC relative calls using IMAGE_REL_I386_REL32
-  // but WinCOFFObjectWriter::RecordRelocation cannot emit them.  Once it does,
-  // the following check for Win32 should be removed.
-  if (Is64Bit || isTargetWin32())
+  if (Is64Bit)
     return false;
   return isTargetELF() || TM.getRelocationModel() == Reloc::Static;
 }

Copy link

github-actions bot commented Mar 4, 2024

⚠️ We detected that you are using a GitHub private e-mail address to contribute to the repo.
Please turn off Keep my email addresses private setting in your account.
See LLVM Discourse for more information.

@AtariDreams AtariDreams force-pushed the wincoff branch 7 times, most recently from 41ab52d to 7ffd0d6 Compare March 5, 2024 23:29
…C-relative calls

This has been true for a while now, but this check has not been removed.
@MaskRay
Copy link
Member

MaskRay commented Mar 7, 2024

This has been true for a while now, but this check has not been removed.

This is not correct. llc -mtriple=i386-pc-win32 -relocation-model=static call-imm.ll -o a.o -filetype=obj has an assertion failure. It might be easier to reject this case like 64-bit.

Copy link
Member

@MaskRay MaskRay left a comment

Choose a reason for hiding this comment

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

.

@AtariDreams AtariDreams closed this Mar 8, 2024
@AtariDreams AtariDreams deleted the wincoff branch March 8, 2024 13:09
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.

None yet

3 participants