Generate correct relocation for external cost centre
authorZejun Wu <watashi@fb.com>
Mon, 15 Oct 2018 17:51:24 +0000 (13:51 -0400)
committerBen Gamari <ben@smart-cactus.org>
Mon, 15 Oct 2018 22:34:17 +0000 (18:34 -0400)
We used to always generate direct access for cost centre labels.  We
fixed this by generating indirect data load for cost centre defined in
external module.

Test Plan:
The added test used to fail with error message
```
/bin/ld.gold: error: T15723B.o: requires dynamic R_X86_64_PC32 reloc
against 'T15723A_foo1_EXPR_cc' which may overflow at runtime; recompile
with -fPIC
```
and now passes.

Also check that `R_X86_64_PC32` is generated for CostCentre from the
same module and `R_X86_64_GOTPCREL` is generated for CostCentre from
external module:
```
$ objdump -rdS T15723B.o
0000000000000028 <T15723B_test_info>:
  28:   48 8d 45 f0             lea    -0x10(%rbp),%rax
  2c:   4c 39 f8                cmp    %r15,%rax
  2f:   72 70                   jb     a1 <T15723B_test_info+0x79>
  31:   48 83 ec 08             sub    $0x8,%rsp
  35:   48 8d 35 00 00 00 00    lea    0x0(%rip),%rsi        # 3c
<T15723B_test_info+0x14>
                        38: R_X86_64_PC32
T15723B_test1_EXPR_cc-0x4
  3c:   49 8b bd 60 03 00 00    mov    0x360(%r13),%rdi
  43:   31 c0                   xor    %eax,%eax
  45:   e8 00 00 00 00          callq  4a <T15723B_test_info+0x22>
                        46: R_X86_64_PLT32      pushCostCentre-0x4
  4a:   48 83 c4 08             add    $0x8,%rsp
  4e:   48 ff 40 30             incq   0x30(%rax)
  52:   49 89 85 60 03 00 00    mov    %rax,0x360(%r13)
  59:   48 83 ec 08             sub    $0x8,%rsp
  5d:   49 8b bd 60 03 00 00    mov    0x360(%r13),%rdi
  64:   48 8b 35 00 00 00 00    mov    0x0(%rip),%rsi        # 6b
<T15723B_test_info+0x43>
                        67: R_X86_64_GOTPCREL   T15723A_foo1_EXPR_cc-0x4
  6b:   31 c0                   xor    %eax,%eax
  6d:   e8 00 00 00 00          callq  72 <T15723B_test_info+0x4a>
                        6e: R_X86_64_PLT32      pushCostCentre-0x4
  72:   48 83 c4 08             add    $0x8,%rsp
  76:   48 ff 40 30             incq   0x30(%rax)
```

Reviewers: simonmar, bgamari

Reviewed By: simonmar

Subscribers: rwbarton, carter

GHC Trac Issues: #15723

Differential Revision: https://phabricator.haskell.org/D5214

compiler/cmm/CLabel.hs
testsuite/tests/codeGen/should_compile/Makefile
testsuite/tests/codeGen/should_compile/T15723.stderr [new file with mode: 0644]
testsuite/tests/codeGen/should_compile/T15723A.hs [new file with mode: 0644]
testsuite/tests/codeGen/should_compile/T15723B.hs [new file with mode: 0644]
testsuite/tests/codeGen/should_compile/all.T

index f07abeb..dbb92e5 100644 (file)
@@ -1019,7 +1019,7 @@ labelDynamic dflags this_mod lbl =
   case lbl of
    -- is the RTS in a DLL or not?
    RtsLabel _ ->
   case lbl of
    -- is the RTS in a DLL or not?
    RtsLabel _ ->
-     (gopt Opt_ExternalDynamicRefs dflags) && (this_pkg /= rtsUnitId)
+     externalDynamicRefs && (this_pkg /= rtsUnitId)
 
    IdLabel n _ _ ->
      isDllName dflags this_mod n
 
    IdLabel n _ _ ->
      isDllName dflags this_mod n
@@ -1028,7 +1028,7 @@ labelDynamic dflags this_mod lbl =
    -- its own shared library.
    CmmLabel pkg _ _
     | os == OSMinGW32 ->
    -- its own shared library.
    CmmLabel pkg _ _
     | os == OSMinGW32 ->
-       (gopt Opt_ExternalDynamicRefs dflags) && (this_pkg /= pkg)
+       externalDynamicRefs && (this_pkg /= pkg)
     | otherwise ->
        gopt Opt_ExternalDynamicRefs dflags
 
     | otherwise ->
        gopt Opt_ExternalDynamicRefs dflags
 
@@ -1048,19 +1048,26 @@ labelDynamic dflags this_mod lbl =
             -- When compiling in the "dyn" way, each package is to be
             -- linked into its own DLL.
             ForeignLabelInPackage pkgId ->
             -- When compiling in the "dyn" way, each package is to be
             -- linked into its own DLL.
             ForeignLabelInPackage pkgId ->
-                (gopt Opt_ExternalDynamicRefs dflags) && (this_pkg /= pkgId)
+                externalDynamicRefs && (this_pkg /= pkgId)
 
        else -- On Mac OS X and on ELF platforms, false positives are OK,
             -- so we claim that all foreign imports come from dynamic
             -- libraries
             True
 
 
        else -- On Mac OS X and on ELF platforms, false positives are OK,
             -- so we claim that all foreign imports come from dynamic
             -- libraries
             True
 
+   CC_Label cc ->
+     externalDynamicRefs && not (ccFromThisModule cc this_mod)
+
+   -- CCS_Label always contains a CostCentre defined in the current module
+   CCS_Label _ -> False
+
    HpcTicksLabel m ->
    HpcTicksLabel m ->
-     (gopt Opt_ExternalDynamicRefs dflags) && this_mod /= m
+     externalDynamicRefs && this_mod /= m
 
    -- Note that DynamicLinkerLabels do NOT require dynamic linking themselves.
    _                 -> False
   where
 
    -- Note that DynamicLinkerLabels do NOT require dynamic linking themselves.
    _                 -> False
   where
+    externalDynamicRefs = gopt Opt_ExternalDynamicRefs dflags
     os = platformOS (targetPlatform dflags)
     this_pkg = moduleUnitId this_mod
 
     os = platformOS (targetPlatform dflags)
     this_pkg = moduleUnitId this_mod
 
index a1fc58f..c072944 100644 (file)
@@ -38,3 +38,8 @@ T14999:
 
 T15196:
        '$(TEST_HC)' $(TEST_HC_OPTS) -c -O -ddump-asm T15196.hs | grep "jp " ; echo $$?
 
 T15196:
        '$(TEST_HC)' $(TEST_HC_OPTS) -c -O -ddump-asm T15196.hs | grep "jp " ; echo $$?
+
+T15723:
+       '$(TEST_HC)' $(TEST_HC_OPTS) -prof -fPIC -fexternal-dynamic-refs -fforce-recomp -O2 -c T15723A.hs -o T15723A.o
+       '$(TEST_HC)' $(TEST_HC_OPTS) -prof -fPIC -fexternal-dynamic-refs -fforce-recomp -O2 -c T15723B.hs -o T15723B.o
+       '$(TEST_HC)' $(TEST_HC_OPTS) -dynamic -shared T15723B.o -o T15723B.so
diff --git a/testsuite/tests/codeGen/should_compile/T15723.stderr b/testsuite/tests/codeGen/should_compile/T15723.stderr
new file mode 100644 (file)
index 0000000..cd2812b
--- /dev/null
@@ -0,0 +1,2 @@
+Warning: -rtsopts and -with-rtsopts have no effect with -shared.
+    Call hs_init_ghc() from your main() function to set these options.
diff --git a/testsuite/tests/codeGen/should_compile/T15723A.hs b/testsuite/tests/codeGen/should_compile/T15723A.hs
new file mode 100644 (file)
index 0000000..aa47ece
--- /dev/null
@@ -0,0 +1,9 @@
+module T15723A where
+
+{-# INLINE foo #-}
+foo :: Int -> Int
+foo x = {-# SCC foo1 #-} bar x
+
+{-# NOINLINE bar #-}
+bar :: Int -> Int
+bar x = x
diff --git a/testsuite/tests/codeGen/should_compile/T15723B.hs b/testsuite/tests/codeGen/should_compile/T15723B.hs
new file mode 100644 (file)
index 0000000..5b7b44d
--- /dev/null
@@ -0,0 +1,6 @@
+module T15723B where
+
+import T15723A
+
+test :: Int -> Int
+test x = {-# SCC test1 #-} foo $ foo x
index a5d5a47..f8e2fb0 100644 (file)
@@ -46,3 +46,8 @@ test('T15196',
   [ unless(arch('x86_64'),skip),
     only_ways('normal'),
   ], run_command, ['$MAKE -s --no-print-directory T15196'])
   [ unless(arch('x86_64'),skip),
     only_ways('normal'),
   ], run_command, ['$MAKE -s --no-print-directory T15196'])
+
+test('T15723',
+  [ unless(have_profiling(), skip),
+    unless(have_dynamic(), skip),
+  ], run_command, ['$MAKE -s --no-print-directory T15723'])