[Git][ghc/ghc][wip/wasm-no-tmp-callee] compiler: avoid saving foreign call target to local when there are no caller-save GlobalRegs

[Cheng Shao (@TerrorJack)]

Cheng Shao pushed to branch wip/wasm-no-tmp-callee at Glasgow Haskell Compiler / GHC


Commits:
328f4a49 by Cheng Shao at 2024-05-22T14:43:11+00:00
compiler: avoid saving foreign call target to local when there are no caller-save GlobalRegs

This patch makes the STG->Cmm backend avoid saving foreign call target
to local when there are no caller-save GlobalRegs.

Since 321941a8ebe25192cdeece723e1058f2f47809ea, when we lower a
foreign call, we unconditionally save the foreign call target to a
temporary local first, then rely on cmmSink to clean it up later,
which only happens with -fcmm-sink (implied by -O) and not in
unoptimized code.

And this is troublesome for the wasm backend NCG, which needs to infer
a foreign call target symbol's type signature from the Cmm call site.
Previously, the NCG has been emitting incorrect type signatures for
unoptimized code, which happens to work with `wasm-ld` most of the
time, but this is never future-proof against upstream toolchain
updates, and it causes horrible breakages when LTO objects are
included in linker input. Hence this patch.

- - - - -


3 changed files:

- compiler/GHC/Driver/Config/StgToCmm.hs
- compiler/GHC/StgToCmm/Config.hs
- compiler/GHC/StgToCmm/Foreign.hs


Changes:

=====================================
compiler/GHC/Driver/Config/StgToCmm.hs
=====================================
@@ -14,6 +14,7 @@ import GHC.Driver.Backend
 import GHC.Driver.Session
 import GHC.Platform
 import GHC.Platform.Profile
+import GHC.Platform.Regs
 import GHC.Utils.Error
 import GHC.Unit.Module
 import GHC.Utils.Outputable
@@ -84,6 +85,8 @@ initStgToCmmConfig dflags mod = StgToCmmConfig
   , stgToCmmAvx2          = isAvx2Enabled                  dflags
   , stgToCmmAvx512f       = isAvx512fEnabled               dflags
   , stgToCmmTickyAP       = gopt Opt_Ticky_AP dflags
+  -- See Note [Saving foreign call target to local]
+  , stgToCmmSaveFCallTargetToLocal = any (callerSaves platform) $ activeStgRegs platform
   } where profile  = targetProfile dflags
           platform = profilePlatform profile
           bk_end  = backend dflags


=====================================
compiler/GHC/StgToCmm/Config.hs
=====================================
@@ -73,6 +73,7 @@ data StgToCmmConfig = StgToCmmConfig
   , stgToCmmAllowWordMul2Instr        :: !Bool   -- ^ Allowed to generate WordMul2 instruction
   , stgToCmmAllowFMAInstr             :: FMASign -> Bool -- ^ Allowed to generate FMA instruction
   , stgToCmmTickyAP                   :: !Bool   -- ^ Disable use of precomputed standard thunks.
+  , stgToCmmSaveFCallTargetToLocal    :: !Bool   -- ^ Save a foreign call target to a Cmm local
   ------------------------------ SIMD flags ------------------------------------
   -- Each of these flags checks vector compatibility with the backend requested
   -- during compilation. In essence, this means checking for @-fllvm@ which is


=====================================
compiler/GHC/StgToCmm/Foreign.hs
=====================================
@@ -277,23 +277,36 @@ load_target_into_temp (ForeignTarget expr conv) = do
 load_target_into_temp other_target@(PrimTarget _) =
   return other_target
 
+-- Note [Saving foreign call target to local]
+-- ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+--
 -- What we want to do here is create a new temporary for the foreign
 -- call argument if it is not safe to use the expression directly,
 -- because the expression mentions caller-saves GlobalRegs (see
 -- Note [Register parameter passing]).
 --
 -- However, we can't pattern-match on the expression here, because
--- this is used in a loop by GHC.Cmm.Parser, and testing the expression
--- results in a black hole.  So we always create a temporary, and rely
--- on GHC.Cmm.Sink to clean it up later.  (Yuck, ToDo).  The generated code
--- ends up being the same, at least for the RTS .cmm code.
+-- this is used in a loop by GHC.Cmm.Parser, and testing the
+-- expression results in a black hole.  So when there exist
+-- caller-saves GlobalRegs, we create a temporary, and rely on
+-- GHC.Cmm.Sink to clean it up later. The generated code ends up being
+-- the same if -fcmm-sink is enabled (implied by -O).
 --
+-- When there doesn't exist caller-save GlobalRegs, keep the original
+-- target in place. This matters for the wasm backend, otherwise it
+-- cannot infer the target symbol's correct foreign function type in
+-- unoptimized Cmm.
 maybe_assign_temp :: CmmExpr -> FCode CmmExpr
 maybe_assign_temp e = do
-  platform <- getPlatform
-  reg <- newTemp (cmmExprType platform e)
-  emitAssign (CmmLocal reg) e
-  return (CmmReg (CmmLocal reg))
+  do_save <- stgToCmmSaveFCallTargetToLocal <$> getStgToCmmConfig
+  if do_save
+    then do
+      platform <- getPlatform
+      reg <- newTemp (cmmExprType platform e)
+      emitAssign (CmmLocal reg) e
+      return (CmmReg (CmmLocal reg))
+    else
+      pure e
 
 -- -----------------------------------------------------------------------------
 -- Save/restore the thread state in the TSO



View it on GitLab: https://gitlab.haskell.org/ghc/ghc/-/commit/328f4a49285ddeda70e9477ee5b220089aa95360

-- 
View it on GitLab: https://gitlab.haskell.org/ghc/ghc/-/commit/328f4a49285ddeda70e9477ee5b220089aa95360
You're receiving this email because of your account on gitlab.haskell.org.


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.haskell.org/pipermail/ghc-commits/attachments/20240522/7f233feb/attachment-0001.html>