[PATCH] D89986: [AIX]ignore the option -fvisibility-inlines-hidden when there is no option -fvisibility=*

2020-10-23 Thread Digger via Phabricator via cfe-commits 
DiggerLin updated this revision to Diff 300383.
DiggerLin added a comment.
Herald added a subscriber: dexonsmith.

address comment


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89986

Files:
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Basic/LangOptions.def
  clang/lib/AST/Decl.cpp
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGen/aix-ignore-xcoff-visibility.cpp
  clang/test/CodeGen/aix-visibility-inlines-hidden.cpp

Index: clang/test/CodeGen/aix-visibility-inlines-hidden.cpp
===
--- /dev/null
+++ clang/test/CodeGen/aix-visibility-inlines-hidden.cpp
@@ -0,0 +1,68 @@
+// REQUIRES: powerpc-registered-target
+
+// RUN: %clang_cc1 -triple powerpc-unknown-aix -mcmodel=large -emit-llvm -o - -x c++ %s  | \
+// RUN: FileCheck -check-prefixes=COMMON-IR,NOVISIBILITY-IR %s
+
+// RUN: %clang_cc1 -triple powerpc-unknown-aix -mcmodel=large -fvisibility-inlines-hidden -emit-llvm -o - -x c++ %s  | \
+// RUN: FileCheck -check-prefixes=COMMON-IR,NOVISIBILITY-IR %s
+
+// RUN: %clang_cc1 -triple powerpc-unknown-aix -mcmodel=large -fvisibility-inlines-hidden \
+// RUN:-fvisibility default -emit-llvm -o - -x c++ %s  | \
+// RUN: FileCheck -check-prefixes=COMMON-IR,VISIBILITY-IR %s
+
+// RUN: %clang_cc1 -triple powerpc-unknown-aix -mcmodel=large -mignore-xcoff-visibility -fvisibility-inlines-hidden \ 
+// RUN:-fvisibility default -emit-llvm -o - -x c++ %s  | \
+// RUN: FileCheck -check-prefixes=COMMON-IR,NOVISIBILITY-IR %s
+
+// RUN: %clang_cc1 -triple powerpc-unknown-aix -mcmodel=large -o - -x c++ -S  %s  |\
+// RUN:   FileCheck --check-prefixes=NOP-ASM,COMMON-ASM %s
+
+// RUN: %clang_cc1 -triple powerpc-unknown-aix -mcmodel=large -fvisibility-inlines-hidden -o - -x c++ -S %s  | \
+// RUN: FileCheck --check-prefixes=NOP-ASM,COMMON-ASM %s
+
+// RUN: %clang_cc1 -triple powerpc-unknown-aix -mcmodel=large -fvisibility-inlines-hidden \
+// RUN:-fvisibility default -o - -x c++ -S %s  | \
+// RUN: FileCheck --check-prefix=COMMON-ASM %s
+
+// RUN: %clang_cc1 -triple powerpc-unknown-aix -mcmodel=large -mignore-xcoff-visibility \
+// RUN:-fvisibility-inlines-hidden  -fvisibility default -o - -x c++ -S %s  | \
+// RUN: FileCheck --check-prefixes=NOP-ASM,COMMON-ASM %s
+int x = 66;
+__attribute__((__noinline__)) inline void f() {
+  x = 55;
+}
+
+#pragma GCC visibility push(hidden)
+__attribute__((__noinline__)) inline void foo() {
+  x = 55;
+}
+#pragma GCC visibility pop
+
+int bar() {
+  f();
+  foo();
+  return x;
+}
+
+// VISIBILITY-IR: define linkonce_odr hidden void @_Z1fv()
+// NOVISIBILITY-IR:   define linkonce_odr void @_Z1fv()
+// COMMON-IR-NEXT:  entry:
+// COMMON-IR-NEXT:store i32 55, i32* @x, align 4
+// COMMON-IR-NEXT:ret void
+
+// VISIBILITY-IR: define linkonce_odr hidden void @_Z3foov()
+// NOVISIBILITY-IR:   define linkonce_odr void @_Z3foov()
+// COMMON-IR-NEXT:  entry:
+// COMMON-IR-NEXT:store i32 55, i32* @x, align 4
+
+// COMMON-ASM: mflr 0
+// COMMON-ASM-NEXT:stw 0, 8(1)
+// COMMON-ASM-NEXT:stwu 1, -64(1)
+// COMMON-ASM-NEXT:bl ._Z1fv
+// NOP-ASM-NEXT:   nop
+// COMMON-ASM-NEXT:bl ._Z3foov
+// NOP-ASM-NEXT:   nop
+
+// RUN: %clang_cc1 -triple powerpc-unknown-aix -mcmodel=large -fvisibility-inlines-hidden  -fvisibility default -o - -x c++ -S %s  | \
+// RUN: FileCheck --check-prefix=CHECK %s
+/// CHECK-NOT: nop
Index: clang/test/CodeGen/aix-ignore-xcoff-visibility.cpp
===
--- clang/test/CodeGen/aix-ignore-xcoff-visibility.cpp
+++ clang/test/CodeGen/aix-ignore-xcoff-visibility.cpp
@@ -17,8 +17,11 @@
 // RUN: %clang_cc1 -triple powerpc-unknown-aix -fvisibility default -o - -x c++ -S %s  | \
 // RUN: FileCheck -check-prefix=VISIBILITY-ASM %s
 
+// RUN: %clang_cc1 -triple powerpc-unknown-aix -emit-llvm -o - -x c++ %s  | \
+// RUN: FileCheck -check-prefix=NOVISIBILITY-IR %s
+
 // RUN: %clang_cc1 -triple powerpc-unknown-aix -mignore-xcoff-visibility -fvisibility default -emit-llvm -o - -x c++ %s  | \
-// RUN: FileCheck -check-prefix=VISIBILITY-IR %s
+// RUN: FileCheck -check-prefix=NOVISIBILITY-IR %s
 
 // RUN: %clang_cc1 -triple powerpc-unknown-aix -fvisibility default -emit-llvm -o - -x c++ %s  | \
 // RUN: FileCheck -check-prefix=VISIBILITY-IR %s
@@ -70,6 +73,15 @@
 // VISIBILITY-IR:define weak_odr protected i32 @_ZN5basicIiE7getdataEv(%class.basic* %this)
 // VISIBILITY-IR:define hidden void @_Z7prambarv()
 
+// NOVISIBILITY-IR:@b = global i32 0
+// NOVISIBILITY-IR:@pramb = global i32 0
+// NOVISIBILITY-IR:define void @_Z5foo_hPi(i32* %p)
+// NOVISIBILITY-IR:declare void @_Z12zoo_extern_hv()
+// NOVISIBILITY-IR:define void @_Z3barv()
+// NOVISIBILITY-IR:define linkonce_odr i32

[PATCH] D89986: [AIX]ignore the option -fvisibility-inlines-hidden when there is no option -fvisibility=*

2020-10-23 Thread Digger via Phabricator via cfe-commits 
DiggerLin added inline comments.



Comment at: clang/test/CodeGen/aix-visibility-inlines-hidden.cpp:30-34
+// COMMON-ASM: mflr 0
+// COMMON-ASM-NEXT:stw 0, 8(1)
+// COMMON-ASM-NEXT:stwu 1, -64(1)
+// COMMON-ASM-NEXT:bl ._Z1fv
+// NOP-ASM-NEXT:   nop

nemanjai wrote:
> rsmith wrote:
> > Generally we strongly prefer for frontend tests to test only the frontend; 
> > in this case, that means testing only the IR that Clang is producing and 
> > not the assembly that comes out of LLVM for that IR. This should also 
> > remove the need to require PPC as a registered target.
> +1
> The asm test can go into `llvm/test/CodeGen/PowerPC`.
If I put the ASM test as separate test case in the llvm/test/CodeGen/PowerPC 
.it will have error as :

ine 1: %clang_cc1: command not found


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89986

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

[PATCH] D89986: [AIX]ignore the option -fvisibility-inlines-hidden when there is no option -fvisibility=*

2020-10-23 Thread Nemanja Ivanovic via Phabricator via cfe-commits 
nemanjai added inline comments.



Comment at: clang/test/CodeGen/aix-visibility-inlines-hidden.cpp:30-34
+// COMMON-ASM: mflr 0
+// COMMON-ASM-NEXT:stw 0, 8(1)
+// COMMON-ASM-NEXT:stwu 1, -64(1)
+// COMMON-ASM-NEXT:bl ._Z1fv
+// NOP-ASM-NEXT:   nop

rsmith wrote:
> Generally we strongly prefer for frontend tests to test only the frontend; in 
> this case, that means testing only the IR that Clang is producing and not the 
> assembly that comes out of LLVM for that IR. This should also remove the need 
> to require PPC as a registered target.
+1
The asm test can go into `llvm/test/CodeGen/PowerPC`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89986

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

[PATCH] D89986: [AIX]ignore the option -fvisibility-inlines-hidden when there is no option -fvisibility=*

2020-10-22 Thread Richard Smith - zygoloid via Phabricator via cfe-commits 
rsmith added inline comments.



Comment at: clang/test/CodeGen/aix-visibility-inlines-hidden.cpp:30-34
+// COMMON-ASM: mflr 0
+// COMMON-ASM-NEXT:stw 0, 8(1)
+// COMMON-ASM-NEXT:stwu 1, -64(1)
+// COMMON-ASM-NEXT:bl ._Z1fv
+// NOP-ASM-NEXT:   nop

Generally we strongly prefer for frontend tests to test only the frontend; in 
this case, that means testing only the IR that Clang is producing and not the 
assembly that comes out of LLVM for that IR. This should also remove the need 
to require PPC as a registered target.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89986

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

[PATCH] D89986: [AIX]ignore the option -fvisibility-inlines-hidden when there is no option -fvisibility=*

2020-10-22 Thread Hubert Tong via Phabricator via cfe-commits 
hubert.reinterpretcast added inline comments.



Comment at: clang/test/CodeGen/aix-visibility-inlines-hidden.cpp:20
+// RUN: FileCheck -check-prefixes=COMMON-IR,VISIBILITY-IR %s
+
+int x = 66;

There's no testing for the interaction with `-mignore-xcoff-visibility`.



Comment at: clang/test/CodeGen/aix-visibility-inlines-hidden.cpp:29
+}
+
+// COMMON-ASM: mflr 0

The general issue is that the `nop` instruction is being omitted even when the 
visibility information is not generated into the object file. This is also a 
problem when the visibility is applied in the source code:
```
int x = 66;
#pragma GCC visibility push(hidden)
__attribute__((__noinline__)) inline void f() {
  x = 55;
}
#pragma GCC visibility pop
int bar() {
  f();
  return x;
}
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89986

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

[PATCH] D89986: [AIX]ignore the option -fvisibility-inlines-hidden when there is no option -fvisibility=*

2020-10-22 Thread Digger via Phabricator via cfe-commits 
DiggerLin created this revision.
DiggerLin added reviewers: jasonliu, hubert.reinterpretcast, sfertile.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
DiggerLin requested review of this revision.

in AIX OS, we will ignore the option -fvisibility-inlines-hidden when there is 
no option -fvisibility=*


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D89986

Files:
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGen/aix-visibility-inlines-hidden.cpp


Index: clang/test/CodeGen/aix-visibility-inlines-hidden.cpp
===
--- /dev/null
+++ clang/test/CodeGen/aix-visibility-inlines-hidden.cpp
@@ -0,0 +1,44 @@
+// REQUIRES: powerpc-registered-target
+
+// RUN: %clang_cc1 -triple powerpc-unknown-aix -mcmodel=large -o - -x c++ -S  
%s  |\
+// RUN:   FileCheck --check-prefixes=NOP-ASM,COMMON-ASM %s
+
+// RUN: %clang_cc1 -triple powerpc-unknown-aix -mcmodel=large 
-fvisibility-inlines-hidden -o - -x c++ -S %s  | \
+// RUN: FileCheck --check-prefixes=NOP-ASM,COMMON-ASM %s
+
+// RUN: %clang_cc1 -triple powerpc-unknown-aix -mcmodel=large 
-fvisibility-inlines-hidden  -fvisibility default -o - -x c++ -S %s  | \
+// RUN: FileCheck --check-prefix=COMMON-ASM %s
+
+// RUN: %clang_cc1 -triple powerpc-unknown-aix -mcmodel=large -emit-llvm -o - 
-x c++ %s  | \
+// RUN: FileCheck -check-prefixes=COMMON-IR,NOVISIBILITY-IR %s
+
+// RUN: %clang_cc1 -triple powerpc-unknown-aix -mcmodel=large 
-fvisibility-inlines-hidden -emit-llvm -o - -x c++ %s  | \
+// RUN: FileCheck -check-prefixes=COMMON-IR,NOVISIBILITY-IR %s
+
+// RUN: %clang_cc1 -triple powerpc-unknown-aix -mcmodel=large 
-fvisibility-inlines-hidden -fvisibility default -emit-llvm -o - -x c++ %s  | \
+// RUN: FileCheck -check-prefixes=COMMON-IR,VISIBILITY-IR %s
+
+int x = 66;
+__attribute__((__noinline__)) inline void f() {
+  x = 55;
+}
+int bar() {
+  f();
+  return x;
+}
+
+// COMMON-ASM: mflr 0
+// COMMON-ASM-NEXT:stw 0, 8(1)
+// COMMON-ASM-NEXT:stwu 1, -64(1)
+// COMMON-ASM-NEXT:bl ._Z1fv
+// NOP-ASM-NEXT:   nop
+
+// VISIBILITY-IR: define linkonce_odr hidden void @_Z1fv()
+// NOVISIBILITY-IR:   define linkonce_odr void @_Z1fv()
+// COMMON-IR-NEXT:  entry:
+// COMMON-IR-NEXT:store i32 55, i32* @x, align 4
+// COMMON-IR-NEXT:ret void
+
+// RUN: %clang_cc1 -triple powerpc-unknown-aix -mcmodel=large 
-fvisibility-inlines-hidden  -fvisibility default -o - -x c++ -S %s  | \
+// RUN: FileCheck --check-prefix=CHECK %s
+/// CHECK-NOT: nop
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -2779,8 +2779,12 @@
 Opts.setTypeVisibilityMode(Opts.getValueVisibilityMode());
   }
 
-  if (Args.hasArg(OPT_fvisibility_inlines_hidden))
-Opts.InlineVisibilityHidden = 1;
+  if (Args.hasArg(OPT_fvisibility_inlines_hidden)) {
+if (T.isOSAIX() && !Args.hasArg(OPT_fvisibility))
+  Opts.InlineVisibilityHidden = 0;
+else
+  Opts.InlineVisibilityHidden = 1;
+  }
 
   if (Args.hasArg(OPT_fvisibility_inlines_hidden_static_local_var))
 Opts.VisibilityInlinesHiddenStaticLocalVar = 1;


Index: clang/test/CodeGen/aix-visibility-inlines-hidden.cpp
===
--- /dev/null
+++ clang/test/CodeGen/aix-visibility-inlines-hidden.cpp
@@ -0,0 +1,44 @@
+// REQUIRES: powerpc-registered-target
+
+// RUN: %clang_cc1 -triple powerpc-unknown-aix -mcmodel=large -o - -x c++ -S  %s  |\
+// RUN:   FileCheck --check-prefixes=NOP-ASM,COMMON-ASM %s
+
+// RUN: %clang_cc1 -triple powerpc-unknown-aix -mcmodel=large -fvisibility-inlines-hidden -o - -x c++ -S %s  | \
+// RUN: FileCheck --check-prefixes=NOP-ASM,COMMON-ASM %s
+
+// RUN: %clang_cc1 -triple powerpc-unknown-aix -mcmodel=large -fvisibility-inlines-hidden  -fvisibility default -o - -x c++ -S %s  | \
+// RUN: FileCheck --check-prefix=COMMON-ASM %s
+
+// RUN: %clang_cc1 -triple powerpc-unknown-aix -mcmodel=large -emit-llvm -o - -x c++ %s  | \
+// RUN: FileCheck -check-prefixes=COMMON-IR,NOVISIBILITY-IR %s
+
+// RUN: %clang_cc1 -triple powerpc-unknown-aix -mcmodel=large -fvisibility-inlines-hidden -emit-llvm -o - -x c++ %s  | \
+// RUN: FileCheck -check-prefixes=COMMON-IR,NOVISIBILITY-IR %s
+
+// RUN: %clang_cc1 -triple powerpc-unknown-aix -mcmodel=large -fvisibility-inlines-hidden -fvisibility default -emit-llvm -o - -x c++ %s  | \
+// RUN: FileCheck -check-prefixes=COMMON-IR,VISIBILITY-IR %s
+
+int x = 66;
+__attribute__((__noinline__)) inline void f() {
+  x = 55;
+}
+int bar() {
+  f();
+  return x;
+}
+
+// COMMON-ASM: mflr 0
+// COMMON-ASM-NEXT:stw 0, 8(1)
+// COMMON-ASM-NEXT:stwu 1, -64(1)
+// COMMON-ASM-NEXT:bl ._Z1fv
+// NOP-ASM-NEXT:   nop
+
+// VISIBILITY-IR: define linkonce_odr hidden void @_Z1fv()
+// NOVISIBILITY-IR:   define linkonce_odr