[PATCH] D81972: [NFC] Cleanup of EmitCXXGlobalInitFunc() and EmitCXXGlobalDtorFunc()

2020-06-18 Thread Xiangling Liao via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rGed1b556954eb: [NFC] Cleanup of EmitCXXGlobalInitFunc() and 
EmitCXXGlobalDtorFunc() (authored by Xiangling_L).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81972

Files:
  clang/lib/CodeGen/CGDeclCXX.cpp


Index: clang/lib/CodeGen/CGDeclCXX.cpp
===
--- clang/lib/CodeGen/CGDeclCXX.cpp
+++ clang/lib/CodeGen/CGDeclCXX.cpp
@@ -533,6 +533,22 @@
   CXXThreadLocals.clear();
 }
 
+static SmallString<128> getTransformedFileName(llvm::Module ) {
+  SmallString<128> FileName = llvm::sys::path::filename(M.getName());
+
+  if (FileName.empty())
+FileName = "";
+
+  for (size_t i = 0; i < FileName.size(); ++i) {
+// Replace everything that's not [a-zA-Z0-9._] with a _. This set happens
+// to be the set of C preprocessing numbers.
+if (!isPreprocessingNumberBody(FileName[i]))
+  FileName[i] = '_';
+  }
+
+  return FileName;
+}
+
 void
 CodeGenModule::EmitCXXGlobalInitFunc() {
   while (!CXXGlobalInits.empty() && !CXXGlobalInits.back())
@@ -577,22 +593,12 @@
 PrioritizedCXXGlobalInits.clear();
   }
 
-  // Include the filename in the symbol name. Including "sub_" matches gcc and
-  // makes sure these symbols appear lexicographically behind the symbols with
-  // priority emitted above.
-  SmallString<128> FileName = llvm::sys::path::filename(getModule().getName());
-  if (FileName.empty())
-FileName = "";
-
-  for (size_t i = 0; i < FileName.size(); ++i) {
-// Replace everything that's not [a-zA-Z0-9._] with a _. This set happens
-// to be the set of C preprocessing numbers.
-if (!isPreprocessingNumberBody(FileName[i]))
-  FileName[i] = '_';
-  }
-
+  // Include the filename in the symbol name. Including "sub_" matches gcc
+  // and makes sure these symbols appear lexicographically behind the symbols
+  // with priority emitted above.
   llvm::Function *Fn = CreateGlobalInitOrDestructFunction(
-  FTy, llvm::Twine("_GLOBAL__sub_I_", FileName), FI);
+  FTy,
+  llvm::Twine("_GLOBAL__sub_I_", getTransformedFileName(getModule())), FI);
 
   CodeGenFunction(*this).GenerateCXXGlobalInitFunc(Fn, CXXGlobalInits);
   AddGlobalCtor(Fn);
@@ -631,6 +637,7 @@
 
   CodeGenFunction(*this).GenerateCXXGlobalDtorsFunc(Fn, CXXGlobalDtors);
   AddGlobalDtor(Fn);
+  CXXGlobalDtors.clear();
 }
 
 /// Emit the code necessary to initialize the given global variable.


Index: clang/lib/CodeGen/CGDeclCXX.cpp
===
--- clang/lib/CodeGen/CGDeclCXX.cpp
+++ clang/lib/CodeGen/CGDeclCXX.cpp
@@ -533,6 +533,22 @@
   CXXThreadLocals.clear();
 }
 
+static SmallString<128> getTransformedFileName(llvm::Module ) {
+  SmallString<128> FileName = llvm::sys::path::filename(M.getName());
+
+  if (FileName.empty())
+FileName = "";
+
+  for (size_t i = 0; i < FileName.size(); ++i) {
+// Replace everything that's not [a-zA-Z0-9._] with a _. This set happens
+// to be the set of C preprocessing numbers.
+if (!isPreprocessingNumberBody(FileName[i]))
+  FileName[i] = '_';
+  }
+
+  return FileName;
+}
+
 void
 CodeGenModule::EmitCXXGlobalInitFunc() {
   while (!CXXGlobalInits.empty() && !CXXGlobalInits.back())
@@ -577,22 +593,12 @@
 PrioritizedCXXGlobalInits.clear();
   }
 
-  // Include the filename in the symbol name. Including "sub_" matches gcc and
-  // makes sure these symbols appear lexicographically behind the symbols with
-  // priority emitted above.
-  SmallString<128> FileName = llvm::sys::path::filename(getModule().getName());
-  if (FileName.empty())
-FileName = "";
-
-  for (size_t i = 0; i < FileName.size(); ++i) {
-// Replace everything that's not [a-zA-Z0-9._] with a _. This set happens
-// to be the set of C preprocessing numbers.
-if (!isPreprocessingNumberBody(FileName[i]))
-  FileName[i] = '_';
-  }
-
+  // Include the filename in the symbol name. Including "sub_" matches gcc
+  // and makes sure these symbols appear lexicographically behind the symbols
+  // with priority emitted above.
   llvm::Function *Fn = CreateGlobalInitOrDestructFunction(
-  FTy, llvm::Twine("_GLOBAL__sub_I_", FileName), FI);
+  FTy,
+  llvm::Twine("_GLOBAL__sub_I_", getTransformedFileName(getModule())), FI);
 
   CodeGenFunction(*this).GenerateCXXGlobalInitFunc(Fn, CXXGlobalInits);
   AddGlobalCtor(Fn);
@@ -631,6 +637,7 @@
 
   CodeGenFunction(*this).GenerateCXXGlobalDtorsFunc(Fn, CXXGlobalDtors);
   AddGlobalDtor(Fn);
+  CXXGlobalDtors.clear();
 }
 
 /// Emit the code necessary to initialize the given global variable.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D81972: [NFC] Cleanup of EmitCXXGlobalInitFunc() and EmitCXXGlobalDtorFunc()

2020-06-18 Thread Hubert Tong via Phabricator via cfe-commits
hubert.reinterpretcast accepted this revision.
hubert.reinterpretcast added a comment.

LGTM; thanks.


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

https://reviews.llvm.org/D81972



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


[PATCH] D81972: [NFC] Cleanup of EmitCXXGlobalInitFunc() and EmitCXXGlobalDtorFunc()

2020-06-18 Thread Xiangling Liao via Phabricator via cfe-commits
Xiangling_L updated this revision to Diff 271661.
Xiangling_L marked 2 inline comments as done.
Xiangling_L added a comment.

Address comments;


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

https://reviews.llvm.org/D81972

Files:
  clang/lib/CodeGen/CGDeclCXX.cpp


Index: clang/lib/CodeGen/CGDeclCXX.cpp
===
--- clang/lib/CodeGen/CGDeclCXX.cpp
+++ clang/lib/CodeGen/CGDeclCXX.cpp
@@ -533,6 +533,22 @@
   CXXThreadLocals.clear();
 }
 
+static SmallString<128> getTransformedFileName(llvm::Module ) {
+  SmallString<128> FileName = llvm::sys::path::filename(M.getName());
+
+  if (FileName.empty())
+FileName = "";
+
+  for (size_t i = 0; i < FileName.size(); ++i) {
+// Replace everything that's not [a-zA-Z0-9._] with a _. This set happens
+// to be the set of C preprocessing numbers.
+if (!isPreprocessingNumberBody(FileName[i]))
+  FileName[i] = '_';
+  }
+
+  return FileName;
+}
+
 void
 CodeGenModule::EmitCXXGlobalInitFunc() {
   while (!CXXGlobalInits.empty() && !CXXGlobalInits.back())
@@ -577,22 +593,12 @@
 PrioritizedCXXGlobalInits.clear();
   }
 
-  // Include the filename in the symbol name. Including "sub_" matches gcc and
-  // makes sure these symbols appear lexicographically behind the symbols with
-  // priority emitted above.
-  SmallString<128> FileName = llvm::sys::path::filename(getModule().getName());
-  if (FileName.empty())
-FileName = "";
-
-  for (size_t i = 0; i < FileName.size(); ++i) {
-// Replace everything that's not [a-zA-Z0-9._] with a _. This set happens
-// to be the set of C preprocessing numbers.
-if (!isPreprocessingNumberBody(FileName[i]))
-  FileName[i] = '_';
-  }
-
+  // Include the filename in the symbol name. Including "sub_" matches gcc
+  // and makes sure these symbols appear lexicographically behind the symbols
+  // with priority emitted above.
   llvm::Function *Fn = CreateGlobalInitOrDestructFunction(
-  FTy, llvm::Twine("_GLOBAL__sub_I_", FileName), FI);
+  FTy,
+  llvm::Twine("_GLOBAL__sub_I_", getTransformedFileName(getModule())), FI);
 
   CodeGenFunction(*this).GenerateCXXGlobalInitFunc(Fn, CXXGlobalInits);
   AddGlobalCtor(Fn);
@@ -631,6 +637,7 @@
 
   CodeGenFunction(*this).GenerateCXXGlobalDtorsFunc(Fn, CXXGlobalDtors);
   AddGlobalDtor(Fn);
+  CXXGlobalDtors.clear();
 }
 
 /// Emit the code necessary to initialize the given global variable.


Index: clang/lib/CodeGen/CGDeclCXX.cpp
===
--- clang/lib/CodeGen/CGDeclCXX.cpp
+++ clang/lib/CodeGen/CGDeclCXX.cpp
@@ -533,6 +533,22 @@
   CXXThreadLocals.clear();
 }
 
+static SmallString<128> getTransformedFileName(llvm::Module ) {
+  SmallString<128> FileName = llvm::sys::path::filename(M.getName());
+
+  if (FileName.empty())
+FileName = "";
+
+  for (size_t i = 0; i < FileName.size(); ++i) {
+// Replace everything that's not [a-zA-Z0-9._] with a _. This set happens
+// to be the set of C preprocessing numbers.
+if (!isPreprocessingNumberBody(FileName[i]))
+  FileName[i] = '_';
+  }
+
+  return FileName;
+}
+
 void
 CodeGenModule::EmitCXXGlobalInitFunc() {
   while (!CXXGlobalInits.empty() && !CXXGlobalInits.back())
@@ -577,22 +593,12 @@
 PrioritizedCXXGlobalInits.clear();
   }
 
-  // Include the filename in the symbol name. Including "sub_" matches gcc and
-  // makes sure these symbols appear lexicographically behind the symbols with
-  // priority emitted above.
-  SmallString<128> FileName = llvm::sys::path::filename(getModule().getName());
-  if (FileName.empty())
-FileName = "";
-
-  for (size_t i = 0; i < FileName.size(); ++i) {
-// Replace everything that's not [a-zA-Z0-9._] with a _. This set happens
-// to be the set of C preprocessing numbers.
-if (!isPreprocessingNumberBody(FileName[i]))
-  FileName[i] = '_';
-  }
-
+  // Include the filename in the symbol name. Including "sub_" matches gcc
+  // and makes sure these symbols appear lexicographically behind the symbols
+  // with priority emitted above.
   llvm::Function *Fn = CreateGlobalInitOrDestructFunction(
-  FTy, llvm::Twine("_GLOBAL__sub_I_", FileName), FI);
+  FTy,
+  llvm::Twine("_GLOBAL__sub_I_", getTransformedFileName(getModule())), FI);
 
   CodeGenFunction(*this).GenerateCXXGlobalInitFunc(Fn, CXXGlobalInits);
   AddGlobalCtor(Fn);
@@ -631,6 +637,7 @@
 
   CodeGenFunction(*this).GenerateCXXGlobalDtorsFunc(Fn, CXXGlobalDtors);
   AddGlobalDtor(Fn);
+  CXXGlobalDtors.clear();
 }
 
 /// Emit the code necessary to initialize the given global variable.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D81972: [NFC] Cleanup of EmitCXXGlobalInitFunc() and EmitCXXGlobalDtorFunc()

2020-06-17 Thread Hubert Tong via Phabricator via cfe-commits
hubert.reinterpretcast added inline comments.



Comment at: clang/lib/CodeGen/CGDeclCXX.cpp:600
+  // with priority emitted above.
+  SmallString<128> FileName;
   llvm::Function *Fn = CreateGlobalInitOrDestructFunction(

In the less complex context of this patch, it seems this should just initialize 
`FileName` with `getTransformedFileName(getModule())` by having 
`getTransformedFileName` return a `SmallString<128>` directly instead of a 
`StringRef` backed by a `SmallString<128>`. I think this would still translate 
well to D74166.


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

https://reviews.llvm.org/D81972



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


[PATCH] D81972: [NFC] Cleanup of EmitCXXGlobalInitFunc() and EmitCXXGlobalDtorFunc()

2020-06-17 Thread Jason Liu via Phabricator via cfe-commits
jasonliu accepted this revision.
jasonliu added a comment.
This revision is now accepted and ready to land.

LGTM.




Comment at: clang/lib/CodeGen/CGDeclCXX.cpp:596
   }
 
+  // Include the filename in the symbol name. Including "sub_" matches gcc

Xiangling_L wrote:
> jasonliu wrote:
> > jasonliu wrote:
> > > jasonliu wrote:
> > > > I think this patch is missing what @hubert.reinterpretcast mentioned in 
> > > > https://reviews.llvm.org/D74166?id=269900#inline-751064
> > > > which is an early return like this:
> > > > 
> > > > ```
> > > >  if (CXXGlobalInits.empty())
> > > > return;
> > > > ```
> > > Please double check the above early return is desired though. It seems 
> > > even when CXXGlobalInits is empty, `GenerateCXXGlobalInitFunc` is trying 
> > > to do a lot of things with `Fn` passed in. Later, we also called 
> > > `AddGlobalCtor(Fn)`. So a lot of behavior changes here, we want to make 
> > > sure it's really 'NFC'.
> > FYI,
> > ```
> > class test {
> >   public:
> > test();
> > };
> > test t1 __attribute__((init_priority (300)));
> > ```
> > Compile with `clang  -target x86_64-apple-darwin10   -emit-llvm `
> > With this change, I guess we will not generate @_GLOBAL__sub_I_d.cpp(), and 
> > also that function will not get added into @llvm.global_ctors.
> > I'm not sure if we really need @_GLOBAL__sub_I_d.cpp() on that platform in 
> > this case. But this change does not really qualify as NFC. 
> > If we need to do this change, a test case is necessary, and we better ask 
> > people who familiar with the platform to confirm it's a desired change. 
> Thanks, I am gonna remove this part and keep it as a NFC patch.
Sure. Please update D74166 accordingly.


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

https://reviews.llvm.org/D81972



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


[PATCH] D81972: [NFC] Cleanup of EmitCXXGlobalInitFunc() and EmitCXXGlobalDtorFunc()

2020-06-17 Thread Xiangling Liao via Phabricator via cfe-commits
Xiangling_L updated this revision to Diff 271492.
Xiangling_L added a comment.

Add the context to the patch;


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

https://reviews.llvm.org/D81972

Files:
  clang/lib/CodeGen/CGDeclCXX.cpp


Index: clang/lib/CodeGen/CGDeclCXX.cpp
===
--- clang/lib/CodeGen/CGDeclCXX.cpp
+++ clang/lib/CodeGen/CGDeclCXX.cpp
@@ -533,6 +533,23 @@
   CXXThreadLocals.clear();
 }
 
+static StringRef getTransformedFileName(llvm::Module ,
+SmallString<128> ) {
+  FileName = llvm::sys::path::filename(M.getName());
+
+  if (FileName.empty())
+FileName = "";
+
+  for (size_t i = 0; i < FileName.size(); ++i) {
+// Replace everything that's not [a-zA-Z0-9._] with a _. This set happens
+// to be the set of C preprocessing numbers.
+if (!isPreprocessingNumberBody(FileName[i]))
+  FileName[i] = '_';
+  }
+
+  return FileName;
+}
+
 void
 CodeGenModule::EmitCXXGlobalInitFunc() {
   while (!CXXGlobalInits.empty() && !CXXGlobalInits.back())
@@ -577,22 +594,15 @@
 PrioritizedCXXGlobalInits.clear();
   }
 
-  // Include the filename in the symbol name. Including "sub_" matches gcc and
-  // makes sure these symbols appear lexicographically behind the symbols with
-  // priority emitted above.
-  SmallString<128> FileName = llvm::sys::path::filename(getModule().getName());
-  if (FileName.empty())
-FileName = "";
-
-  for (size_t i = 0; i < FileName.size(); ++i) {
-// Replace everything that's not [a-zA-Z0-9._] with a _. This set happens
-// to be the set of C preprocessing numbers.
-if (!isPreprocessingNumberBody(FileName[i]))
-  FileName[i] = '_';
-  }
-
+  // Include the filename in the symbol name. Including "sub_" matches gcc
+  // and makes sure these symbols appear lexicographically behind the symbols
+  // with priority emitted above.
+  SmallString<128> FileName;
   llvm::Function *Fn = CreateGlobalInitOrDestructFunction(
-  FTy, llvm::Twine("_GLOBAL__sub_I_", FileName), FI);
+  FTy,
+  llvm::Twine("_GLOBAL__sub_I_",
+  getTransformedFileName(getModule(), FileName)),
+  FI);
 
   CodeGenFunction(*this).GenerateCXXGlobalInitFunc(Fn, CXXGlobalInits);
   AddGlobalCtor(Fn);
@@ -631,6 +641,7 @@
 
   CodeGenFunction(*this).GenerateCXXGlobalDtorsFunc(Fn, CXXGlobalDtors);
   AddGlobalDtor(Fn);
+  CXXGlobalDtors.clear();
 }
 
 /// Emit the code necessary to initialize the given global variable.


Index: clang/lib/CodeGen/CGDeclCXX.cpp
===
--- clang/lib/CodeGen/CGDeclCXX.cpp
+++ clang/lib/CodeGen/CGDeclCXX.cpp
@@ -533,6 +533,23 @@
   CXXThreadLocals.clear();
 }
 
+static StringRef getTransformedFileName(llvm::Module ,
+SmallString<128> ) {
+  FileName = llvm::sys::path::filename(M.getName());
+
+  if (FileName.empty())
+FileName = "";
+
+  for (size_t i = 0; i < FileName.size(); ++i) {
+// Replace everything that's not [a-zA-Z0-9._] with a _. This set happens
+// to be the set of C preprocessing numbers.
+if (!isPreprocessingNumberBody(FileName[i]))
+  FileName[i] = '_';
+  }
+
+  return FileName;
+}
+
 void
 CodeGenModule::EmitCXXGlobalInitFunc() {
   while (!CXXGlobalInits.empty() && !CXXGlobalInits.back())
@@ -577,22 +594,15 @@
 PrioritizedCXXGlobalInits.clear();
   }
 
-  // Include the filename in the symbol name. Including "sub_" matches gcc and
-  // makes sure these symbols appear lexicographically behind the symbols with
-  // priority emitted above.
-  SmallString<128> FileName = llvm::sys::path::filename(getModule().getName());
-  if (FileName.empty())
-FileName = "";
-
-  for (size_t i = 0; i < FileName.size(); ++i) {
-// Replace everything that's not [a-zA-Z0-9._] with a _. This set happens
-// to be the set of C preprocessing numbers.
-if (!isPreprocessingNumberBody(FileName[i]))
-  FileName[i] = '_';
-  }
-
+  // Include the filename in the symbol name. Including "sub_" matches gcc
+  // and makes sure these symbols appear lexicographically behind the symbols
+  // with priority emitted above.
+  SmallString<128> FileName;
   llvm::Function *Fn = CreateGlobalInitOrDestructFunction(
-  FTy, llvm::Twine("_GLOBAL__sub_I_", FileName), FI);
+  FTy,
+  llvm::Twine("_GLOBAL__sub_I_",
+  getTransformedFileName(getModule(), FileName)),
+  FI);
 
   CodeGenFunction(*this).GenerateCXXGlobalInitFunc(Fn, CXXGlobalInits);
   AddGlobalCtor(Fn);
@@ -631,6 +641,7 @@
 
   CodeGenFunction(*this).GenerateCXXGlobalDtorsFunc(Fn, CXXGlobalDtors);
   AddGlobalDtor(Fn);
+  CXXGlobalDtors.clear();
 }
 
 /// Emit the code necessary to initialize the given global variable.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D81972: [NFC] Cleanup of EmitCXXGlobalInitFunc() and EmitCXXGlobalDtorFunc()

2020-06-17 Thread Jason Liu via Phabricator via cfe-commits
jasonliu added a comment.

New diff does not have context available.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81972



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


[PATCH] D81972: [NFC] Cleanup of EmitCXXGlobalInitFunc() and EmitCXXGlobalDtorFunc()

2020-06-17 Thread Xiangling Liao via Phabricator via cfe-commits
Xiangling_L added inline comments.



Comment at: clang/lib/CodeGen/CGDeclCXX.cpp:596
   }
 
+  // Include the filename in the symbol name. Including "sub_" matches gcc

jasonliu wrote:
> jasonliu wrote:
> > jasonliu wrote:
> > > I think this patch is missing what @hubert.reinterpretcast mentioned in 
> > > https://reviews.llvm.org/D74166?id=269900#inline-751064
> > > which is an early return like this:
> > > 
> > > ```
> > >  if (CXXGlobalInits.empty())
> > > return;
> > > ```
> > Please double check the above early return is desired though. It seems even 
> > when CXXGlobalInits is empty, `GenerateCXXGlobalInitFunc` is trying to do a 
> > lot of things with `Fn` passed in. Later, we also called 
> > `AddGlobalCtor(Fn)`. So a lot of behavior changes here, we want to make 
> > sure it's really 'NFC'.
> FYI,
> ```
> class test {
>   public:
> test();
> };
> test t1 __attribute__((init_priority (300)));
> ```
> Compile with `clang  -target x86_64-apple-darwin10   -emit-llvm `
> With this change, I guess we will not generate @_GLOBAL__sub_I_d.cpp(), and 
> also that function will not get added into @llvm.global_ctors.
> I'm not sure if we really need @_GLOBAL__sub_I_d.cpp() on that platform in 
> this case. But this change does not really qualify as NFC. 
> If we need to do this change, a test case is necessary, and we better ask 
> people who familiar with the platform to confirm it's a desired change. 
Thanks, I am gonna remove this part and keep it as a NFC patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81972



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


[PATCH] D81972: [NFC] Cleanup of EmitCXXGlobalInitFunc() and EmitCXXGlobalDtorFunc()

2020-06-17 Thread Xiangling Liao via Phabricator via cfe-commits
Xiangling_L updated this revision to Diff 271486.
Xiangling_L marked 3 inline comments as done.
Xiangling_L added a comment.

Remove early return part;


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81972

Files:
  clang/lib/CodeGen/CGDeclCXX.cpp


Index: clang/lib/CodeGen/CGDeclCXX.cpp
===
--- clang/lib/CodeGen/CGDeclCXX.cpp
+++ clang/lib/CodeGen/CGDeclCXX.cpp
@@ -533,6 +533,23 @@
   CXXThreadLocals.clear();
 }
 
+static StringRef getTransformedFileName(llvm::Module ,
+SmallString<128> ) {
+  FileName = llvm::sys::path::filename(M.getName());
+
+  if (FileName.empty())
+FileName = "";
+
+  for (size_t i = 0; i < FileName.size(); ++i) {
+// Replace everything that's not [a-zA-Z0-9._] with a _. This set happens
+// to be the set of C preprocessing numbers.
+if (!isPreprocessingNumberBody(FileName[i]))
+  FileName[i] = '_';
+  }
+
+  return FileName;
+}
+
 void
 CodeGenModule::EmitCXXGlobalInitFunc() {
   while (!CXXGlobalInits.empty() && !CXXGlobalInits.back())
@@ -577,22 +594,15 @@
 PrioritizedCXXGlobalInits.clear();
   }
 
-  // Include the filename in the symbol name. Including "sub_" matches gcc and
-  // makes sure these symbols appear lexicographically behind the symbols with
-  // priority emitted above.
-  SmallString<128> FileName = llvm::sys::path::filename(getModule().getName());
-  if (FileName.empty())
-FileName = "";
-
-  for (size_t i = 0; i < FileName.size(); ++i) {
-// Replace everything that's not [a-zA-Z0-9._] with a _. This set happens
-// to be the set of C preprocessing numbers.
-if (!isPreprocessingNumberBody(FileName[i]))
-  FileName[i] = '_';
-  }
-
+  // Include the filename in the symbol name. Including "sub_" matches gcc
+  // and makes sure these symbols appear lexicographically behind the symbols
+  // with priority emitted above.
+  SmallString<128> FileName;
   llvm::Function *Fn = CreateGlobalInitOrDestructFunction(
-  FTy, llvm::Twine("_GLOBAL__sub_I_", FileName), FI);
+  FTy,
+  llvm::Twine("_GLOBAL__sub_I_",
+  getTransformedFileName(getModule(), FileName)),
+  FI);
 
   CodeGenFunction(*this).GenerateCXXGlobalInitFunc(Fn, CXXGlobalInits);
   AddGlobalCtor(Fn);
@@ -631,6 +641,7 @@
 
   CodeGenFunction(*this).GenerateCXXGlobalDtorsFunc(Fn, CXXGlobalDtors);
   AddGlobalDtor(Fn);
+  CXXGlobalDtors.clear();
 }
 
 /// Emit the code necessary to initialize the given global variable.


Index: clang/lib/CodeGen/CGDeclCXX.cpp
===
--- clang/lib/CodeGen/CGDeclCXX.cpp
+++ clang/lib/CodeGen/CGDeclCXX.cpp
@@ -533,6 +533,23 @@
   CXXThreadLocals.clear();
 }
 
+static StringRef getTransformedFileName(llvm::Module ,
+SmallString<128> ) {
+  FileName = llvm::sys::path::filename(M.getName());
+
+  if (FileName.empty())
+FileName = "";
+
+  for (size_t i = 0; i < FileName.size(); ++i) {
+// Replace everything that's not [a-zA-Z0-9._] with a _. This set happens
+// to be the set of C preprocessing numbers.
+if (!isPreprocessingNumberBody(FileName[i]))
+  FileName[i] = '_';
+  }
+
+  return FileName;
+}
+
 void
 CodeGenModule::EmitCXXGlobalInitFunc() {
   while (!CXXGlobalInits.empty() && !CXXGlobalInits.back())
@@ -577,22 +594,15 @@
 PrioritizedCXXGlobalInits.clear();
   }
 
-  // Include the filename in the symbol name. Including "sub_" matches gcc and
-  // makes sure these symbols appear lexicographically behind the symbols with
-  // priority emitted above.
-  SmallString<128> FileName = llvm::sys::path::filename(getModule().getName());
-  if (FileName.empty())
-FileName = "";
-
-  for (size_t i = 0; i < FileName.size(); ++i) {
-// Replace everything that's not [a-zA-Z0-9._] with a _. This set happens
-// to be the set of C preprocessing numbers.
-if (!isPreprocessingNumberBody(FileName[i]))
-  FileName[i] = '_';
-  }
-
+  // Include the filename in the symbol name. Including "sub_" matches gcc
+  // and makes sure these symbols appear lexicographically behind the symbols
+  // with priority emitted above.
+  SmallString<128> FileName;
   llvm::Function *Fn = CreateGlobalInitOrDestructFunction(
-  FTy, llvm::Twine("_GLOBAL__sub_I_", FileName), FI);
+  FTy,
+  llvm::Twine("_GLOBAL__sub_I_",
+  getTransformedFileName(getModule(), FileName)),
+  FI);
 
   CodeGenFunction(*this).GenerateCXXGlobalInitFunc(Fn, CXXGlobalInits);
   AddGlobalCtor(Fn);
@@ -631,6 +641,7 @@
 
   CodeGenFunction(*this).GenerateCXXGlobalDtorsFunc(Fn, CXXGlobalDtors);
   AddGlobalDtor(Fn);
+  CXXGlobalDtors.clear();
 }
 
 /// Emit the code necessary to initialize the given global variable.
___
cfe-commits mailing list

[PATCH] D81972: [NFC] Cleanup of EmitCXXGlobalInitFunc() and EmitCXXGlobalDtorFunc()

2020-06-17 Thread Jason Liu via Phabricator via cfe-commits
jasonliu added inline comments.



Comment at: clang/lib/CodeGen/CGDeclCXX.cpp:596
   }
 
+  // Include the filename in the symbol name. Including "sub_" matches gcc

jasonliu wrote:
> jasonliu wrote:
> > I think this patch is missing what @hubert.reinterpretcast mentioned in 
> > https://reviews.llvm.org/D74166?id=269900#inline-751064
> > which is an early return like this:
> > 
> > ```
> >  if (CXXGlobalInits.empty())
> > return;
> > ```
> Please double check the above early return is desired though. It seems even 
> when CXXGlobalInits is empty, `GenerateCXXGlobalInitFunc` is trying to do a 
> lot of things with `Fn` passed in. Later, we also called `AddGlobalCtor(Fn)`. 
> So a lot of behavior changes here, we want to make sure it's really 'NFC'.
FYI,
```
class test {
  public:
test();
};
test t1 __attribute__((init_priority (300)));
```
Compile with `clang  -target x86_64-apple-darwin10   -emit-llvm `
With this change, I guess we will not generate @_GLOBAL__sub_I_d.cpp(), and 
also that function will not get added into @llvm.global_ctors.
I'm not sure if we really need @_GLOBAL__sub_I_d.cpp() on that platform in this 
case. But this change does not really qualify as NFC. 
If we need to do this change, a test case is necessary, and we better ask 
people who familiar with the platform to confirm it's a desired change. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81972



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


[PATCH] D81972: [NFC] Cleanup of EmitCXXGlobalInitFunc() and EmitCXXGlobalDtorFunc()

2020-06-17 Thread Jason Liu via Phabricator via cfe-commits
jasonliu added inline comments.



Comment at: clang/lib/CodeGen/CGDeclCXX.cpp:596
   }
 
+  // Include the filename in the symbol name. Including "sub_" matches gcc

jasonliu wrote:
> I think this patch is missing what @hubert.reinterpretcast mentioned in 
> https://reviews.llvm.org/D74166?id=269900#inline-751064
> which is an early return like this:
> 
> ```
>  if (CXXGlobalInits.empty())
> return;
> ```
Please double check the above early return is desired though. It seems even 
when CXXGlobalInits is empty, `GenerateCXXGlobalInitFunc` is trying to do a lot 
of things with `Fn` passed in. Later, we also called `AddGlobalCtor(Fn)`. So a 
lot of behavior changes here, we want to make sure it's really 'NFC'.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81972



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


[PATCH] D81972: [NFC] Cleanup of EmitCXXGlobalInitFunc() and EmitCXXGlobalDtorFunc()

2020-06-17 Thread Xiangling Liao via Phabricator via cfe-commits
Xiangling_L updated this revision to Diff 271389.
Xiangling_L marked an inline comment as done.
Xiangling_L added a comment.

Minor change;


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81972

Files:
  clang/lib/CodeGen/CGDeclCXX.cpp


Index: clang/lib/CodeGen/CGDeclCXX.cpp
===
--- clang/lib/CodeGen/CGDeclCXX.cpp
+++ clang/lib/CodeGen/CGDeclCXX.cpp
@@ -533,6 +533,23 @@
   CXXThreadLocals.clear();
 }
 
+static StringRef getTransformedFileName(llvm::Module ,
+SmallString<128> ) {
+  FileName = llvm::sys::path::filename(M.getName());
+
+  if (FileName.empty())
+FileName = "";
+
+  for (size_t i = 0; i < FileName.size(); ++i) {
+// Replace everything that's not [a-zA-Z0-9._] with a _. This set happens
+// to be the set of C preprocessing numbers.
+if (!isPreprocessingNumberBody(FileName[i]))
+  FileName[i] = '_';
+  }
+
+  return FileName;
+}
+
 void
 CodeGenModule::EmitCXXGlobalInitFunc() {
   while (!CXXGlobalInits.empty() && !CXXGlobalInits.back())
@@ -577,22 +594,18 @@
 PrioritizedCXXGlobalInits.clear();
   }
 
-  // Include the filename in the symbol name. Including "sub_" matches gcc and
-  // makes sure these symbols appear lexicographically behind the symbols with
-  // priority emitted above.
-  SmallString<128> FileName = llvm::sys::path::filename(getModule().getName());
-  if (FileName.empty())
-FileName = "";
-
-  for (size_t i = 0; i < FileName.size(); ++i) {
-// Replace everything that's not [a-zA-Z0-9._] with a _. This set happens
-// to be the set of C preprocessing numbers.
-if (!isPreprocessingNumberBody(FileName[i]))
-  FileName[i] = '_';
-  }
+  if (CXXGlobalInits.empty())
+return;
 
+  // Include the filename in the symbol name. Including "sub_" matches gcc
+  // and makes sure these symbols appear lexicographically behind the symbols
+  // with priority emitted above.
+  SmallString<128> FileName;
   llvm::Function *Fn = CreateGlobalInitOrDestructFunction(
-  FTy, llvm::Twine("_GLOBAL__sub_I_", FileName), FI);
+  FTy,
+  llvm::Twine("_GLOBAL__sub_I_",
+  getTransformedFileName(getModule(), FileName)),
+  FI);
 
   CodeGenFunction(*this).GenerateCXXGlobalInitFunc(Fn, CXXGlobalInits);
   AddGlobalCtor(Fn);
@@ -631,6 +644,7 @@
 
   CodeGenFunction(*this).GenerateCXXGlobalDtorsFunc(Fn, CXXGlobalDtors);
   AddGlobalDtor(Fn);
+  CXXGlobalDtors.clear();
 }
 
 /// Emit the code necessary to initialize the given global variable.


Index: clang/lib/CodeGen/CGDeclCXX.cpp
===
--- clang/lib/CodeGen/CGDeclCXX.cpp
+++ clang/lib/CodeGen/CGDeclCXX.cpp
@@ -533,6 +533,23 @@
   CXXThreadLocals.clear();
 }
 
+static StringRef getTransformedFileName(llvm::Module ,
+SmallString<128> ) {
+  FileName = llvm::sys::path::filename(M.getName());
+
+  if (FileName.empty())
+FileName = "";
+
+  for (size_t i = 0; i < FileName.size(); ++i) {
+// Replace everything that's not [a-zA-Z0-9._] with a _. This set happens
+// to be the set of C preprocessing numbers.
+if (!isPreprocessingNumberBody(FileName[i]))
+  FileName[i] = '_';
+  }
+
+  return FileName;
+}
+
 void
 CodeGenModule::EmitCXXGlobalInitFunc() {
   while (!CXXGlobalInits.empty() && !CXXGlobalInits.back())
@@ -577,22 +594,18 @@
 PrioritizedCXXGlobalInits.clear();
   }
 
-  // Include the filename in the symbol name. Including "sub_" matches gcc and
-  // makes sure these symbols appear lexicographically behind the symbols with
-  // priority emitted above.
-  SmallString<128> FileName = llvm::sys::path::filename(getModule().getName());
-  if (FileName.empty())
-FileName = "";
-
-  for (size_t i = 0; i < FileName.size(); ++i) {
-// Replace everything that's not [a-zA-Z0-9._] with a _. This set happens
-// to be the set of C preprocessing numbers.
-if (!isPreprocessingNumberBody(FileName[i]))
-  FileName[i] = '_';
-  }
+  if (CXXGlobalInits.empty())
+return;
 
+  // Include the filename in the symbol name. Including "sub_" matches gcc
+  // and makes sure these symbols appear lexicographically behind the symbols
+  // with priority emitted above.
+  SmallString<128> FileName;
   llvm::Function *Fn = CreateGlobalInitOrDestructFunction(
-  FTy, llvm::Twine("_GLOBAL__sub_I_", FileName), FI);
+  FTy,
+  llvm::Twine("_GLOBAL__sub_I_",
+  getTransformedFileName(getModule(), FileName)),
+  FI);
 
   CodeGenFunction(*this).GenerateCXXGlobalInitFunc(Fn, CXXGlobalInits);
   AddGlobalCtor(Fn);
@@ -631,6 +644,7 @@
 
   CodeGenFunction(*this).GenerateCXXGlobalDtorsFunc(Fn, CXXGlobalDtors);
   AddGlobalDtor(Fn);
+  CXXGlobalDtors.clear();
 }
 
 /// Emit the code necessary to initialize the given global variable.

[PATCH] D81972: [NFC] Cleanup of EmitCXXGlobalInitFunc() and EmitCXXGlobalDtorFunc()

2020-06-16 Thread Jason Liu via Phabricator via cfe-commits
jasonliu added inline comments.



Comment at: clang/lib/CodeGen/CGDeclCXX.cpp:596
   }
 
+  // Include the filename in the symbol name. Including "sub_" matches gcc

I think this patch is missing what @hubert.reinterpretcast mentioned in 
https://reviews.llvm.org/D74166?id=269900#inline-751064
which is an early return like this:

```
 if (CXXGlobalInits.empty())
return;
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81972



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


[PATCH] D81972: [NFC] Cleanup of EmitCXXGlobalInitFunc() and EmitCXXGlobalDtorFunc()

2020-06-16 Thread Xiangling Liao via Phabricator via cfe-commits
Xiangling_L created this revision.
Xiangling_L added reviewers: jasonliu, hubert.reinterpretcast.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
Xiangling_L edited the summary of this revision.

Tidy up some code of `EmitCXXGlobalInitFunc()` and `EmitCXXGlobalDtorFunc()`as 
the pre-work of [AIX][Frontend] Static init implementation for AIX considering 
no priority [https://reviews.llvm.org/D74166] patch.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D81972

Files:
  clang/lib/CodeGen/CGDeclCXX.cpp


Index: clang/lib/CodeGen/CGDeclCXX.cpp
===
--- clang/lib/CodeGen/CGDeclCXX.cpp
+++ clang/lib/CodeGen/CGDeclCXX.cpp
@@ -533,6 +533,23 @@
   CXXThreadLocals.clear();
 }
 
+static StringRef getTransformedFileName(llvm::Module ,
+SmallString<128> ) {
+  FileName = llvm::sys::path::filename(M.getName());
+
+  if (FileName.empty())
+FileName = "";
+
+  for (size_t i = 0; i < FileName.size(); ++i) {
+// Replace everything that's not [a-zA-Z0-9._] with a _. This set happens
+// to be the set of C preprocessing numbers.
+if (!isPreprocessingNumberBody(FileName[i]))
+  FileName[i] = '_';
+  }
+
+  return FileName;
+}
+
 void
 CodeGenModule::EmitCXXGlobalInitFunc() {
   while (!CXXGlobalInits.empty() && !CXXGlobalInits.back())
@@ -577,22 +594,15 @@
 PrioritizedCXXGlobalInits.clear();
   }
 
-  // Include the filename in the symbol name. Including "sub_" matches gcc and
-  // makes sure these symbols appear lexicographically behind the symbols with
-  // priority emitted above.
-  SmallString<128> FileName = llvm::sys::path::filename(getModule().getName());
-  if (FileName.empty())
-FileName = "";
-
-  for (size_t i = 0; i < FileName.size(); ++i) {
-// Replace everything that's not [a-zA-Z0-9._] with a _. This set happens
-// to be the set of C preprocessing numbers.
-if (!isPreprocessingNumberBody(FileName[i]))
-  FileName[i] = '_';
-  }
-
+  // Include the filename in the symbol name. Including "sub_" matches gcc
+  // and makes sure these symbols appear lexicographically behind the symbols
+  // with priority emitted above.
+  SmallString<128> FileName;
   llvm::Function *Fn = CreateGlobalInitOrDestructFunction(
-  FTy, llvm::Twine("_GLOBAL__sub_I_", FileName), FI);
+  FTy,
+  llvm::Twine("_GLOBAL__sub_I_",
+  getTransformedFileName(getModule(), FileName)),
+  FI);
 
   CodeGenFunction(*this).GenerateCXXGlobalInitFunc(Fn, CXXGlobalInits);
   AddGlobalCtor(Fn);
@@ -631,6 +641,7 @@
 
   CodeGenFunction(*this).GenerateCXXGlobalDtorsFunc(Fn, CXXGlobalDtors);
   AddGlobalDtor(Fn);
+  CXXGlobalDtors.clear();
 }
 
 /// Emit the code necessary to initialize the given global variable.


Index: clang/lib/CodeGen/CGDeclCXX.cpp
===
--- clang/lib/CodeGen/CGDeclCXX.cpp
+++ clang/lib/CodeGen/CGDeclCXX.cpp
@@ -533,6 +533,23 @@
   CXXThreadLocals.clear();
 }
 
+static StringRef getTransformedFileName(llvm::Module ,
+SmallString<128> ) {
+  FileName = llvm::sys::path::filename(M.getName());
+
+  if (FileName.empty())
+FileName = "";
+
+  for (size_t i = 0; i < FileName.size(); ++i) {
+// Replace everything that's not [a-zA-Z0-9._] with a _. This set happens
+// to be the set of C preprocessing numbers.
+if (!isPreprocessingNumberBody(FileName[i]))
+  FileName[i] = '_';
+  }
+
+  return FileName;
+}
+
 void
 CodeGenModule::EmitCXXGlobalInitFunc() {
   while (!CXXGlobalInits.empty() && !CXXGlobalInits.back())
@@ -577,22 +594,15 @@
 PrioritizedCXXGlobalInits.clear();
   }
 
-  // Include the filename in the symbol name. Including "sub_" matches gcc and
-  // makes sure these symbols appear lexicographically behind the symbols with
-  // priority emitted above.
-  SmallString<128> FileName = llvm::sys::path::filename(getModule().getName());
-  if (FileName.empty())
-FileName = "";
-
-  for (size_t i = 0; i < FileName.size(); ++i) {
-// Replace everything that's not [a-zA-Z0-9._] with a _. This set happens
-// to be the set of C preprocessing numbers.
-if (!isPreprocessingNumberBody(FileName[i]))
-  FileName[i] = '_';
-  }
-
+  // Include the filename in the symbol name. Including "sub_" matches gcc
+  // and makes sure these symbols appear lexicographically behind the symbols
+  // with priority emitted above.
+  SmallString<128> FileName;
   llvm::Function *Fn = CreateGlobalInitOrDestructFunction(
-  FTy, llvm::Twine("_GLOBAL__sub_I_", FileName), FI);
+  FTy,
+  llvm::Twine("_GLOBAL__sub_I_",
+  getTransformedFileName(getModule(), FileName)),
+  FI);
 
   CodeGenFunction(*this).GenerateCXXGlobalInitFunc(Fn, CXXGlobalInits);
   AddGlobalCtor(Fn);
@@ -631,6 +641,7 @@
 
   CodeGenFunction(*this).GenerateCXXGlobalDtorsFunc(Fn, CXXGlobalDtors);