Skip to content

[Driver][UEFI] Enable Microsoft extensions #121875

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

Merged
merged 2 commits into from
Jan 8, 2025

Conversation

Prabhuk
Copy link
Contributor

@Prabhuk Prabhuk commented Jan 7, 2025

Set "-fms-extensions" for UEFI targets.

@llvmbot llvmbot added clang Clang issues not falling into any other category clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' libc labels Jan 7, 2025
@llvmbot
Copy link
Member

llvmbot commented Jan 7, 2025

@llvm/pr-subscribers-libc

@llvm/pr-subscribers-clang-driver

Author: Prabhuk (Prabhuk)

Changes
  • [libc] Keep framepointers for baremetal
  • [Driver][UEFI] Enable Microsoft extensions

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

2 Files Affected:

  • (modified) clang/lib/Driver/ToolChains/Clang.cpp (+2-1)
  • (modified) libc/config/baremetal/config.json (-5)
diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp
index a0002371da2f1b..03fdd92e00e316 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -5109,6 +5109,7 @@ void Clang::ConstructJob(Compilation &C, const JobAction &JA,
   const llvm::Triple *AuxTriple =
       (IsCuda || IsHIP) ? TC.getAuxTriple() : nullptr;
   bool IsWindowsMSVC = RawTriple.isWindowsMSVCEnvironment();
+  bool IsUEFI = RawTriple.isUEFI();
   bool IsIAMCU = RawTriple.isOSIAMCU();
 
   // Adjust IsWindowsXYZ for CUDA/HIP/SYCL compilations.  Even when compiling in
@@ -7252,7 +7253,7 @@ void Clang::ConstructJob(Compilation &C, const JobAction &JA,
 
   // -fms-extensions=0 is default.
   if (Args.hasFlag(options::OPT_fms_extensions, options::OPT_fno_ms_extensions,
-                   IsWindowsMSVC))
+                   IsWindowsMSVC || IsUEFI))
     CmdArgs.push_back("-fms-extensions");
 
   // -fms-compatibility=0 is default.
diff --git a/libc/config/baremetal/config.json b/libc/config/baremetal/config.json
index 85e80879d498e6..08c581d1c68226 100644
--- a/libc/config/baremetal/config.json
+++ b/libc/config/baremetal/config.json
@@ -30,10 +30,5 @@
     "LIBC_CONF_MATH_OPTIMIZATIONS": {
       "value": "(LIBC_MATH_SKIP_ACCURATE_PASS | LIBC_MATH_SMALL_TABLES)"
     }
-  },
-  "codegen": {
-    "LIBC_CONF_KEEP_FRAME_POINTER": {
-      "value": false
-    }
   }
 }

@llvmbot
Copy link
Member

llvmbot commented Jan 7, 2025

@llvm/pr-subscribers-clang

Author: Prabhuk (Prabhuk)

Changes
  • [libc] Keep framepointers for baremetal
  • [Driver][UEFI] Enable Microsoft extensions

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

2 Files Affected:

  • (modified) clang/lib/Driver/ToolChains/Clang.cpp (+2-1)
  • (modified) libc/config/baremetal/config.json (-5)
diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp
index a0002371da2f1b..03fdd92e00e316 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -5109,6 +5109,7 @@ void Clang::ConstructJob(Compilation &C, const JobAction &JA,
   const llvm::Triple *AuxTriple =
       (IsCuda || IsHIP) ? TC.getAuxTriple() : nullptr;
   bool IsWindowsMSVC = RawTriple.isWindowsMSVCEnvironment();
+  bool IsUEFI = RawTriple.isUEFI();
   bool IsIAMCU = RawTriple.isOSIAMCU();
 
   // Adjust IsWindowsXYZ for CUDA/HIP/SYCL compilations.  Even when compiling in
@@ -7252,7 +7253,7 @@ void Clang::ConstructJob(Compilation &C, const JobAction &JA,
 
   // -fms-extensions=0 is default.
   if (Args.hasFlag(options::OPT_fms_extensions, options::OPT_fno_ms_extensions,
-                   IsWindowsMSVC))
+                   IsWindowsMSVC || IsUEFI))
     CmdArgs.push_back("-fms-extensions");
 
   // -fms-compatibility=0 is default.
diff --git a/libc/config/baremetal/config.json b/libc/config/baremetal/config.json
index 85e80879d498e6..08c581d1c68226 100644
--- a/libc/config/baremetal/config.json
+++ b/libc/config/baremetal/config.json
@@ -30,10 +30,5 @@
     "LIBC_CONF_MATH_OPTIMIZATIONS": {
       "value": "(LIBC_MATH_SKIP_ACCURATE_PASS | LIBC_MATH_SMALL_TABLES)"
     }
-  },
-  "codegen": {
-    "LIBC_CONF_KEEP_FRAME_POINTER": {
-      "value": false
-    }
   }
 }

@SchrodingerZhu
Copy link
Contributor

Why is the libc patch included in this one?

@Prabhuk Prabhuk marked this pull request as draft January 7, 2025 17:13
@Prabhuk
Copy link
Contributor Author

Prabhuk commented Jan 7, 2025

Why is the libc patch included in this one?

Thank you! It was a mistake. I'll upload the right version and marking this PR as draft until then.

Enable microsoft extensions for UEFI targets.
@Prabhuk Prabhuk force-pushed the enable_ms_extensions branch from 8294d83 to d0377d3 Compare January 7, 2025 20:16
@Prabhuk Prabhuk marked this pull request as ready for review January 7, 2025 20:32
@Prabhuk Prabhuk requested review from frobtech and petrhosek January 7, 2025 20:32
@petrhosek petrhosek changed the title enable ms extensions [Driver][UEFI] Enable Microsoft extensions Jan 7, 2025
@petrhosek
Copy link
Member

Can you include a test as well?

Copy link
Contributor

@frobtech frobtech left a comment

Choose a reason for hiding this comment

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

LGTM given a driver test as Petr mentioned.

@Prabhuk
Copy link
Contributor Author

Prabhuk commented Jan 8, 2025

@RossComputerGuy FYI

@Prabhuk
Copy link
Contributor Author

Prabhuk commented Jan 8, 2025

Can you include a test as well?

@petrhosek Updated the test to check for "-fms-extensions" flag. PTAL

@RossComputerGuy
Copy link
Member

@RossComputerGuy FYI

Already have seen this, I'm not sure I understand what the flag does exactly. I've got LLVM libc PR subscriptions so I'll see any future PR's.

@Prabhuk Prabhuk merged commit bb9785a into llvm:main Jan 8, 2025
6 of 7 checks passed
@Prabhuk
Copy link
Contributor Author

Prabhuk commented Jan 8, 2025

@RossComputerGuy FYI

Already have seen this, I'm not sure I understand what the flag does exactly. I've got LLVM libc PR subscriptions so I'll see any future PR's.

This flag enables support for PECOFF specific pragmas.

@llvm-ci
Copy link
Collaborator

llvm-ci commented Jan 8, 2025

LLVM Buildbot has detected a new failure on builder clang-debian-cpp20 running on clang-debian-cpp20 while building clang at step 6 "test-build-unified-tree-check-all".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/108/builds/7886

Here is the relevant piece of the build log for the reference
Step 6 (test-build-unified-tree-check-all) failure: test (failure)
******************** TEST 'LLVM :: ExecutionEngine/JITLink/x86-64/COFF_pdata_no_strip.s' FAILED ********************
Exit Code: 1

Command Output (stderr):
--
RUN: at line 1: /vol/worker/clang-debian-cpp20/clang-debian-cpp20/build/bin/llvm-mc -filetype=obj -triple=x86_64-windows-msvc /vol/worker/clang-debian-cpp20/clang-debian-cpp20/llvm-project/llvm/test/ExecutionEngine/JITLink/x86-64/COFF_pdata_no_strip.s -o /vol/worker/clang-debian-cpp20/clang-debian-cpp20/build/test/ExecutionEngine/JITLink/x86-64/Output/COFF_pdata_no_strip.s.tmp
+ /vol/worker/clang-debian-cpp20/clang-debian-cpp20/build/bin/llvm-mc -filetype=obj -triple=x86_64-windows-msvc /vol/worker/clang-debian-cpp20/clang-debian-cpp20/llvm-project/llvm/test/ExecutionEngine/JITLink/x86-64/COFF_pdata_no_strip.s -o /vol/worker/clang-debian-cpp20/clang-debian-cpp20/build/test/ExecutionEngine/JITLink/x86-64/Output/COFF_pdata_no_strip.s.tmp
RUN: at line 2 has no command after substitutions
RUN: at line 3: /vol/worker/clang-debian-cpp20/clang-debian-cpp20/build/bin/llvm-jitlink -abs __ImageBase=0xdeadbeaf -noexec /vol/worker/clang-debian-cpp20/clang-debian-cpp20/build/test/ExecutionEngine/JITLink/x86-64/Output/COFF_pdata_no_strip.s.tmp  -slab-allocate 100Kb -slab-address 0xfff00000 -slab-page-size 4096  -show-graphs='.*' -noexec 2>&1 | /vol/worker/clang-debian-cpp20/clang-debian-cpp20/build/bin/FileCheck /vol/worker/clang-debian-cpp20/clang-debian-cpp20/llvm-project/llvm/test/ExecutionEngine/JITLink/x86-64/COFF_pdata_no_strip.s
+ /vol/worker/clang-debian-cpp20/clang-debian-cpp20/build/bin/FileCheck /vol/worker/clang-debian-cpp20/clang-debian-cpp20/llvm-project/llvm/test/ExecutionEngine/JITLink/x86-64/COFF_pdata_no_strip.s
+ /vol/worker/clang-debian-cpp20/clang-debian-cpp20/build/bin/llvm-jitlink -abs __ImageBase=0xdeadbeaf -noexec /vol/worker/clang-debian-cpp20/clang-debian-cpp20/build/test/ExecutionEngine/JITLink/x86-64/Output/COFF_pdata_no_strip.s.tmp -slab-allocate 100Kb -slab-address 0xfff00000 -slab-page-size 4096 '-show-graphs=.*' -noexec
/vol/worker/clang-debian-cpp20/clang-debian-cpp20/llvm-project/llvm/test/ExecutionEngine/JITLink/x86-64/COFF_pdata_no_strip.s:8:10: error: CHECK: expected string not found in input
# CHECK: section .xdata:
         ^
<stdin>:1:1: note: scanning from here
llvm-jitlink error: Resource tracker 0x590338357aa0 became defunct
^
<stdin>:2:192: note: possible intended match here
llvm-jitlink: /vol/worker/clang-debian-cpp20/clang-debian-cpp20/llvm-project/llvm/include/llvm/ExecutionEngine/Orc/SymbolStringPool.h:285: llvm::orc::SymbolStringPool::~SymbolStringPool(): Assertion `Pool.empty() && "Dangling references at pool destruction time"' failed.
                                                                                                                                                                                               ^

Input file: <stdin>
Check file: /vol/worker/clang-debian-cpp20/clang-debian-cpp20/llvm-project/llvm/test/ExecutionEngine/JITLink/x86-64/COFF_pdata_no_strip.s

-dump-input=help explains the following input dump.

Input was:
<<<<<<
           1: llvm-jitlink error: Resource tracker 0x590338357aa0 became defunct 
check:8'0     X~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ error: no match found
           2: llvm-jitlink: /vol/worker/clang-debian-cpp20/clang-debian-cpp20/llvm-project/llvm/include/llvm/ExecutionEngine/Orc/SymbolStringPool.h:285: llvm::orc::SymbolStringPool::~SymbolStringPool(): Assertion `Pool.empty() && "Dangling references at pool destruction time"' failed. 
check:8'0     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
check:8'1                                                                                                                                                                                                    ?                                                                                 possible intended match
           3: PLEASE submit a bug report to https://github.com/llvm/llvm-project/issues/ and include the crash backtrace. 
check:8'0     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
           4: Stack dump: 
check:8'0     ~~~~~~~~~~~~
           5: 0. Program arguments: /vol/worker/clang-debian-cpp20/clang-debian-cpp20/build/bin/llvm-jitlink -abs __ImageBase=0xdeadbeaf -noexec /vol/worker/clang-debian-cpp20/clang-debian-cpp20/build/test/ExecutionEngine/JITLink/x86-64/Output/COFF_pdata_no_strip.s.tmp -slab-allocate 100Kb -slab-address 0xfff00000 -slab-page-size 4096 -show-graphs=.* -noexec 
check:8'0     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
           6:  #0 0x0000590334a1ee68 llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) (/vol/worker/clang-debian-cpp20/clang-debian-cpp20/build/bin/llvm-jitlink+0xeade68) 
check:8'0     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
           7:  #1 0x0000590334a1c95d llvm::sys::RunSignalHandlers() (/vol/worker/clang-debian-cpp20/clang-debian-cpp20/build/bin/llvm-jitlink+0xeab95d) 
check:8'0     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
           .
           .
           .
>>>>>>

--

...

@Prabhuk Prabhuk deleted the enable_ms_extensions branch May 14, 2025 21:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' clang Clang issues not falling into any other category libc
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants