[PATCH] D87451: add new option -mignore-xcoff-visibility

2020-10-08 Thread Dave Green via Phabricator via cfe-commits 
dmgreen added a comment.

Hello. I added a power-pc REQUIRES clause to the new clang test here in 
a15bd0bfc20c2b2955c59450a67b6e8efe89c708 
. Hope 
that looks OK.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D87451/new/

https://reviews.llvm.org/D87451

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D87451: add new option -mignore-xcoff-visibility

2020-10-08 Thread Digger via Phabricator via cfe-commits 
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
DiggerLin marked an inline comment as done.
Closed by commit rG92bca1284308: [AIX] add new option -mignore-xcoff-visibility 
(authored by DiggerLin).

Changed prior to commit:
  https://reviews.llvm.org/D87451?vs=296267=296953#toc

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D87451/new/

https://reviews.llvm.org/D87451

Files:
  clang/docs/ClangCommandLineReference.rst
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGen/aix-ignore-xcoff-visibility.cpp
  clang/test/Driver/ignore-xcoff-visibility.cpp
  llvm/include/llvm/CodeGen/CommandFlags.h
  llvm/include/llvm/Target/TargetMachine.h
  llvm/include/llvm/Target/TargetOptions.h
  llvm/lib/CodeGen/CommandFlags.cpp
  llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp
  llvm/test/CodeGen/PowerPC/aix-ignore-xcoff-visibility.ll

Index: llvm/test/CodeGen/PowerPC/aix-ignore-xcoff-visibility.ll
===
--- /dev/null
+++ llvm/test/CodeGen/PowerPC/aix-ignore-xcoff-visibility.ll
@@ -0,0 +1,48 @@
+; RUN: llc -verify-machineinstrs -mtriple powerpc-ibm-aix-xcoff -mcpu=pwr4 \
+; RUN: -mattr=-altivec  < %s | \
+; RUN:   FileCheck --check-prefix=VISIBILITY-ASM %s
+; RUN: llc -verify-machineinstrs -mtriple powerpc-ibm-aix-xcoff -mcpu=pwr4 \
+; RUN: -mattr=-altivec -ignore-xcoff-visibility < %s | \
+; RUN:   FileCheck --check-prefix=IGNOREVISIBILITY-ASM %s
+; RUN: llc -verify-machineinstrs -mtriple powerpc64-ibm-aix-xcoff -mcpu=pwr4 \
+; RUN: -mattr=-altivec < %s | \
+; RUN:   FileCheck --check-prefix=VISIBILITY-ASM %s
+; RUN: llc -verify-machineinstrs -mtriple powerpc64-ibm-aix-xcoff -mcpu=pwr4 \
+; RUN: -mattr=-altivec -ignore-xcoff-visibility < %s | \
+; RUN:   FileCheck --check-prefix=IGNOREVISIBILITY-ASM %s
+
+@foo_p = global void ()* @zoo_extern_h, align 4
+@b = protected global i32 0, align 4
+
+define hidden void @foo_h(i32* %p) {
+entry:
+  %p.addr = alloca i32*, align 4
+  store i32* %p, i32** %p.addr, align 4
+  %0 = load i32*, i32** %p.addr, align 4
+  %1 = load i32, i32* %0, align 4
+  %inc = add nsw i32 %1, 1
+  store i32 %inc, i32* %0, align 4
+  ret void
+}
+
+declare hidden void @zoo_extern_h()
+
+define protected void @bar() {
+entry:
+  call void @foo_h(i32* @b)
+  %0 = load void ()*, void ()** @foo_p, align 4
+  call void %0()
+  ret void
+}
+
+; VISIBILITY-ASM: .globl  foo_h[DS],hidden
+; VISIBILITY-ASM: .globl  .foo_h,hidden
+; VISIBILITY-ASM: .globl  bar[DS],protected
+; VISIBILITY-ASM: .globl  .bar,protected
+; VISIBILITY-ASM: .globl  b,protected
+
+; IGNOREVISIBILITY-ASM: .globl  foo_h[DS]
+; IGNOREVISIBILITY-ASM: .globl  .foo_h
+; IGNOREVISIBILITY-ASM: .globl  bar[DS]
+; IGNOREVISIBILITY-ASM: .globl  .bar
+; IGNOREVISIBILITY-ASM: .globl  b
Index: llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp
===
--- llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp
+++ llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp
@@ -1702,17 +1702,19 @@
   assert(LinkageAttr != MCSA_Invalid && "LinkageAttr should not MCSA_Invalid.");
 
   MCSymbolAttr VisibilityAttr = MCSA_Invalid;
-  switch (GV->getVisibility()) {
+  if (!TM.getIgnoreXCOFFVisibility()) {
+switch (GV->getVisibility()) {
 
-  // TODO: "exported" and "internal" Visibility needs to go here.
-  case GlobalValue::DefaultVisibility:
-break;
-  case GlobalValue::HiddenVisibility:
-VisibilityAttr = MAI->getHiddenVisibilityAttr();
-break;
-  case GlobalValue::ProtectedVisibility:
-VisibilityAttr = MAI->getProtectedVisibilityAttr();
-break;
+// TODO: "exported" and "internal" Visibility needs to go here.
+case GlobalValue::DefaultVisibility:
+  break;
+case GlobalValue::HiddenVisibility:
+  VisibilityAttr = MAI->getHiddenVisibilityAttr();
+  break;
+case GlobalValue::ProtectedVisibility:
+  VisibilityAttr = MAI->getProtectedVisibilityAttr();
+  break;
+}
   }
 
   OutStreamer->emitXCOFFSymbolLinkageWithVisibility(GVSym, LinkageAttr,
Index: llvm/lib/CodeGen/CommandFlags.cpp
===
--- llvm/lib/CodeGen/CommandFlags.cpp
+++ llvm/lib/CodeGen/CommandFlags.cpp
@@ -74,6 +74,7 @@
 CGOPT(bool, RelaxELFRelocations)
 CGOPT_EXP(bool, DataSections)
 CGOPT_EXP(bool, FunctionSections)
+CGOPT(bool, IgnoreXCOFFVisibility)
 CGOPT(std::string, BBSections)
 CGOPT(unsigned, TLSSize)
 CGOPT(bool, EmulatedTLS)
@@ -333,6 +334,13 @@
   cl::init(false));
   CGBINDOPT(FunctionSections);
 
+  static cl::opt IgnoreXCOFFVisibility(
+  "ignore-xcoff-visibility",
+  cl::desc("Not emit the visibility attribute for asm in AIX OS or give "
+   "all

[PATCH] D87451: add new option -mignore-xcoff-visibility

2020-10-07 Thread Digger via Phabricator via cfe-commits 
DiggerLin marked an inline comment as done.
DiggerLin added inline comments.



Comment at: llvm/include/llvm/Target/TargetOptions.h:126
   FunctionSections(false), DataSections(false),
-  UniqueSectionNames(true), UniqueBasicBlockSectionNames(false),
-  TrapUnreachable(false), NoTrapAfterNoreturn(false), TLSSize(0),
-  EmulatedTLS(false), ExplicitEmulatedTLS(false), EnableIPRA(false),
+  IgnoreXCOFFVisibility(false), UniqueSectionNames(true),
+  UniqueBasicBlockSectionNames(false), TrapUnreachable(false),

jasonliu wrote:
> Should the default be true for this option?
> As this is an XCOFF only option, and the default for clang is true, so it 
> would be better for llc to match as well?
we can implement it in another separate patch. 


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D87451/new/

https://reviews.llvm.org/D87451

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D87451: add new option -mignore-xcoff-visibility

2020-10-07 Thread Jason Liu via Phabricator via cfe-commits 
jasonliu added inline comments.



Comment at: llvm/include/llvm/Target/TargetOptions.h:126
   FunctionSections(false), DataSections(false),
-  UniqueSectionNames(true), UniqueBasicBlockSectionNames(false),
-  TrapUnreachable(false), NoTrapAfterNoreturn(false), TLSSize(0),
-  EmulatedTLS(false), ExplicitEmulatedTLS(false), EnableIPRA(false),
+  IgnoreXCOFFVisibility(false), UniqueSectionNames(true),
+  UniqueBasicBlockSectionNames(false), TrapUnreachable(false),

Should the default be true for this option?
As this is an XCOFF only option, and the default for clang is true, so it would 
be better for llc to match as well?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D87451/new/

https://reviews.llvm.org/D87451

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D87451: add new option -mignore-xcoff-visibility

2020-10-06 Thread David Tenty via Phabricator via cfe-commits 
daltenty accepted this revision.
daltenty added a comment.
This revision is now accepted and ready to land.

LGTM, other minor nit


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D87451/new/

https://reviews.llvm.org/D87451

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D87451: add new option -mignore-xcoff-visibility

2020-10-06 Thread David Tenty via Phabricator via cfe-commits 
daltenty added inline comments.



Comment at: clang/docs/ClangCommandLineReference.rst:2622
+
+Do not emit any visibility attribute for asm on AIX or give all symbols 
'unspecified' visibility in xcoff object file (XCOFF only)
+

nit: plural. capitalization.



Comment at: clang/include/clang/Driver/Options.td:2566
+def mignore_xcoff_visibility : Flag<["-"], "mignore-xcoff-visibility">, 
Group,
+HelpText<"Not emit the visibility attribute for asm in AIX OS or give all 
symbols 'unspecified' visibility in xcoff object file">,
+  Flags<[CC1Option]>;





Comment at: llvm/lib/CodeGen/CommandFlags.cpp:340
+  cl::desc("Not emit the visibility attribute for asm in AIX OS or give "
+   "all symbols 'unspecified' visibility in xcoff object file"),
+  cl::init(false));

nit: capitalize XCOFF


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D87451/new/

https://reviews.llvm.org/D87451

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D87451: add new option -mignore-xcoff-visibility

2020-10-06 Thread David Tenty via Phabricator via cfe-commits 
daltenty added inline comments.



Comment at: llvm/lib/CodeGen/CommandFlags.cpp:339
+  "ignore-xcoff-visibility",
+  cl::desc("Not emit the visibility attribute for asm in AIX OS or give "
+   "all symbols 'unspecified' visibility in xcoff object file"),

nit: match description above


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D87451/new/

https://reviews.llvm.org/D87451

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits