[PATCH] D70876: [clang-tidy] Add spuriously-wake-up-functions check

[Balázs Benics via Phabricator via cfe-commits]

steakhal added inline comments.



Comment at: clang-tools-extra/docs/clang-tidy/checks/list.rst:300-301
 
+   `cert-con36-c `_, `bugprone-spuriously-wake-up-functions 
`_, "Yes"
+   `cert-con54-cpp `_, 
`bugprone-spuriously-wake-up-functions 
`_, "Yes"
`cert-dcl03-c `_, `misc-static-assert 
`_, "Yes"

In case the checker does not offer automatic fixes, remove `"Yes"` from the 
given table entries.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D70876



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

[PATCH] D74463: Add new check avoid-adjacent-unrelated-parameters-of-the-same-type

[Kim Viggedal via Phabricator via cfe-commits]

vingeldal updated this revision to Diff 244072.
vingeldal added a comment.

Updating D74463 : Add new check 
avoid-adjacent-unrelated-parameters-of-the-same-type

- Changed commit message to start with [clang-tidy]


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74463

Files:
  
clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidAdjacentParametersOfTheSameTypeCheck.cpp
  
clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidAdjacentParametersOfTheSameTypeCheck.h
  clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt
  clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-avoid-adjacent-parameters-of-the-same-type.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-avoid-adjacent-parameters-of-the-same-type.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-avoid-adjacent-parameters-of-the-same-type.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-avoid-adjacent-parameters-of-the-same-type.cpp
@@ -0,0 +1,57 @@
+// RUN: %check_clang_tidy %s cppcoreguidelines-avoid-adjacent-parameters-of-the-same-type %t
+
+void func_no_parameter();
+
+void func_parameter_argument(int arg1);
+
+void func_int_int_array(int arg1, int arg2[]);
+
+void func_int_int_reference(int arg1, int );
+
+void func_int_int_pointer(int arg1, int *arg2);
+
+void func_int_const_int(int arg1, const int arg2);
+
+void func_int_unsigned_int(int arg1, unsigned int arg2);
+
+void func_int_long_int(int arg1, long int arg2);
+
+void func_int_float(int arg1, float arg2);
+
+void func_more_than_two_different(char arg1, int arg2, float arg3);
+
+enum DummyEnum { first,
+ second };
+void func_int_and_enum(int arg1, DummyEnum arg2);
+
+template 
+void func_template(T arg1, int arg2);
+
+template 
+void func_template_adjacent_int(T arg1, int arg2, int arg3);
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'func_template_adjacent_int' has adjacent parameters of the same type. If the order of these parameters matter consider rewriting this to avoid a mixup of parameters. [cppcoreguidelines-avoid-adjacent-parameters-of-the-same-type]
+
+template 
+void func_template_adjacent_template_type(T arg1, T arg2);
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'func_template_adjacent_template_type' has adjacent parameters of the same type. If the order of these parameters matter consider rewriting this to avoid a mixup of parameters. [cppcoreguidelines-avoid-adjacent-parameters-of-the-same-type]
+
+void func_two_adjacent_int(int arg1, int arg2);
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'func_two_adjacent_int' has adjacent parameters of the same type. If the order of these parameters matter consider rewriting this to avoid a mixup of parameters. [cppcoreguidelines-avoid-adjacent-parameters-of-the-same-type]
+
+void func_adjacent_int_more_than_two(int arg1, int arg2, int int_3);
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'func_adjacent_int_more_than_two' has adjacent parameters of the same type. If the order of these parameters matter consider rewriting this to avoid a mixup of parameters. [cppcoreguidelines-avoid-adjacent-parameters-of-the-same-type]
+
+void func_adjacent_int_not_all_same(float arg1, int arg2, int int_3);
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'func_adjacent_int_not_all_same' has adjacent parameters of the same type. If the order of these parameters matter consider rewriting this to avoid a mixup of parameters. [cppcoreguidelines-avoid-adjacent-parameters-of-the-same-type]
+
+void func_adjacent_float(float arg1, float arg2);
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'func_adjacent_float' has adjacent parameters of the same type. If the order of these parameters matter consider rewriting this to avoid a mixup of parameters. [cppcoreguidelines-avoid-adjacent-parameters-of-the-same-type]
+
+class DummyClass {
+public:
+  void func_member1();
+  void func_member2(int arg1, int arg2, float arg3);
+  static void func_member3(int arg1, int arg2);
+};
+// CHECK-MESSAGES: :[[@LINE-3]]:10: warning: function 'func_member2' has adjacent parameters of the same type. If the order of these parameters matter consider rewriting this to avoid a mixup of parameters. [cppcoreguidelines-avoid-adjacent-parameters-of-the-same-type]
+// CHECK-MESSAGES: :[[@LINE-3]]:17: warning: function 'func_member3' has adjacent parameters of the same type. If the order of these parameters matter consider rewriting this to avoid a mixup of parameters. [cppcoreguidelines-avoid-adjacent-parameters-of-the-same-type]
\ No newline at end of file
Index: 

[PATCH] D74463: Add new check avoid-adjacent-unrelated-parameters-of-the-same-type

[Kim Viggedal via Phabricator via cfe-commits]

vingeldal created this revision.
Herald added subscribers: cfe-commits, kbarton, mgorny, nemanjai.
Herald added a project: clang.

This check is part of the C++ Core Guidelines, rule I.24
https://github.com/vingeldal/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#Ri-unrelated


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D74463

Files:
  
clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidAdjacentParametersOfTheSameTypeCheck.cpp
  
clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidAdjacentParametersOfTheSameTypeCheck.h
  clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt
  clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-avoid-adjacent-parameters-of-the-same-type.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-avoid-adjacent-parameters-of-the-same-type.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-avoid-adjacent-parameters-of-the-same-type.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-avoid-adjacent-parameters-of-the-same-type.cpp
@@ -0,0 +1,57 @@
+// RUN: %check_clang_tidy %s cppcoreguidelines-avoid-adjacent-parameters-of-the-same-type %t
+
+void func_no_parameter();
+
+void func_parameter_argument(int arg1);
+
+void func_int_int_array(int arg1, int arg2[]);
+
+void func_int_int_reference(int arg1, int );
+
+void func_int_int_pointer(int arg1, int *arg2);
+
+void func_int_const_int(int arg1, const int arg2);
+
+void func_int_unsigned_int(int arg1, unsigned int arg2);
+
+void func_int_long_int(int arg1, long int arg2);
+
+void func_int_float(int arg1, float arg2);
+
+void func_more_than_two_different(char arg1, int arg2, float arg3);
+
+enum DummyEnum { first,
+ second };
+void func_int_and_enum(int arg1, DummyEnum arg2);
+
+template 
+void func_template(T arg1, int arg2);
+
+template 
+void func_template_adjacent_int(T arg1, int arg2, int arg3);
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'func_template_adjacent_int' has adjacent parameters of the same type. If the order of these parameters matter consider rewriting this to avoid a mixup of parameters. [cppcoreguidelines-avoid-adjacent-parameters-of-the-same-type]
+
+template 
+void func_template_adjacent_template_type(T arg1, T arg2);
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'func_template_adjacent_template_type' has adjacent parameters of the same type. If the order of these parameters matter consider rewriting this to avoid a mixup of parameters. [cppcoreguidelines-avoid-adjacent-parameters-of-the-same-type]
+
+void func_two_adjacent_int(int arg1, int arg2);
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'func_two_adjacent_int' has adjacent parameters of the same type. If the order of these parameters matter consider rewriting this to avoid a mixup of parameters. [cppcoreguidelines-avoid-adjacent-parameters-of-the-same-type]
+
+void func_adjacent_int_more_than_two(int arg1, int arg2, int int_3);
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'func_adjacent_int_more_than_two' has adjacent parameters of the same type. If the order of these parameters matter consider rewriting this to avoid a mixup of parameters. [cppcoreguidelines-avoid-adjacent-parameters-of-the-same-type]
+
+void func_adjacent_int_not_all_same(float arg1, int arg2, int int_3);
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'func_adjacent_int_not_all_same' has adjacent parameters of the same type. If the order of these parameters matter consider rewriting this to avoid a mixup of parameters. [cppcoreguidelines-avoid-adjacent-parameters-of-the-same-type]
+
+void func_adjacent_float(float arg1, float arg2);
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'func_adjacent_float' has adjacent parameters of the same type. If the order of these parameters matter consider rewriting this to avoid a mixup of parameters. [cppcoreguidelines-avoid-adjacent-parameters-of-the-same-type]
+
+class DummyClass {
+public:
+  void func_member1();
+  void func_member2(int arg1, int arg2, float arg3);
+  static void func_member3(int arg1, int arg2);
+};
+// CHECK-MESSAGES: :[[@LINE-3]]:10: warning: function 'func_member2' has adjacent parameters of the same type. If the order of these parameters matter consider rewriting this to avoid a mixup of parameters. [cppcoreguidelines-avoid-adjacent-parameters-of-the-same-type]
+// CHECK-MESSAGES: :[[@LINE-3]]:17: warning: function 'func_member3' has adjacent parameters of the same type. If the order of these parameters matter consider rewriting this to avoid a mixup of parameters. [cppcoreguidelines-avoid-adjacent-parameters-of-the-same-type]
\ No newline at end of file
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst

[PATCH] D74456: Reland "[Support] make report_fatal_error `abort` instead of `exit`"

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

`ninja check-llvm check-clang` passed on a powerpc64le machine.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74456



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

[PATCH] D74456: Reland "[Support] make report_fatal_error `abort` instead of `exit`"

[Yuanfang Chen via Phabricator via cfe-commits]

ychen added a comment.

In D74456#1871238 , @MaskRay wrote:

> `ninja check-llvm check-clang` passed on a powerpc64le machine.


Thanks for the review. Fingers crossed sanitizer bots not complain.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74456



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

[PATCH] D72579: Evaluate __{c11_,}atomic_is_lock_free to 0 (avoid libcall) if larger than MaxAtomicPromoteWidth

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

From https://gcc.gnu.org/ml/gcc/2020-02/msg00116.html

> We discussed this on IRC in #gcc. There was no motivation to change GCC. The 
> platform that wants to avoid the libatomic dependency doesn't use GCC anyway. 
> Relying on not getting the libatomic dependency seems fragile (it only works 
> for a specific codebase, and if some other is_lock_free check is added to the 
> codebase, the libatomic dependency will return anyway).

So they will not extend the interface. Clang has `getMaxAtomicPromoteWidth()`. 
Do we need the extension? I'm happy with either way.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72579



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

[PATCH] D74447: [Clang] After integrated-cc1, ignore -disable-free when there are more than one job in the queue

[Alexandre Ganea via Phabricator via cfe-commits]

aganea updated this revision to Diff 244055.
aganea marked 3 inline comments as done.
aganea added a comment.

As suggested by Reid.


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

https://reviews.llvm.org/D74447

Files:
  clang/include/clang/Driver/Job.h
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/Job.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/cc1-spawnprocess.c
  clang/tools/driver/cc1_main.cpp

Index: clang/tools/driver/cc1_main.cpp
===
--- clang/tools/driver/cc1_main.cpp
+++ clang/tools/driver/cc1_main.cpp
@@ -259,6 +259,7 @@
   // FIXME(ibiryukov): make profilerOutput flush in destructor instead.
   profilerOutput->flush();
   llvm::timeTraceProfilerCleanup();
+  Clang->clearOutputFiles(false);
 }
   }
 
Index: clang/test/Driver/cc1-spawnprocess.c
===
--- clang/test/Driver/cc1-spawnprocess.c
+++ clang/test/Driver/cc1-spawnprocess.c
@@ -20,3 +20,48 @@
 
 // YES: (in-process)
 // NO-NOT: (in-process)
+
+// The following tests ensure that only the last integrated-cc1 has -disable-free
+
+// RUN: echo 'int main() { return f() + g(); }' > %t1.cpp
+// RUN: echo 'int f() { return 1; }' > %t2.cpp
+// RUN: echo 'int g() { return 2; }' > %t3.cpp
+
+// Only one TU, one job, thus no cleanup is needed.
+// RUN: %clang -fintegrated-cc1 -c %t1.cpp -### 2>&1 | FileCheck %s --check-prefix=CLEAN
+// CLEAN: cc1
+// CLEAN-SAME: -disable-free
+
+// Three jobs, only the last invocation will skip clean up.
+// RUN: %clang -fintegrated-cc1 -c %t1.cpp %t2.cpp %t3.cpp -### 2>&1 | FileCheck %s --check-prefix=NOCLEAN-LAST
+// NOCLEAN-LAST: cc1
+// NOCLEAN-LAST-NOT: -disable-free
+// NOCLEAN-LAST-SAME: {{.*}}1.cpp
+// NOCLEAN-LAST: cc1
+// NOCLEAN-LAST-NOT: -disable-free
+// NOCLEAN-LAST-SAME: {{.*}}2.cpp
+// NOCLEAN-LAST: cc1
+// NOCLEAN-LAST-SAME: -disable-free
+// NOCLEAN-LAST-SAME: {{.*}}3.cpp
+
+// Four jobs, no invocation will skip clean up because the last job is linking.
+// RUN: %clang -fintegrated-cc1 %t1.cpp %t2.cpp %t3.cpp -### 2>&1 | FileCheck %s --check-prefix=ALL
+// ALL-NOT: -disable-free
+
+// In no-integrated-cc1 mode, no cleanup is needed, because we're running the
+// CC1 tool in a secondary process and the process' heap will be released
+// anyway when the process ends.
+// RUN: %clang -fno-integrated-cc1 -c %t1.cpp -### 2>&1 | FileCheck %s --check-prefix=CLEAN
+
+// RUN: %clang -fno-integrated-cc1 -c %t1.cpp %t2.cpp %t3.cpp -### 2>&1 | FileCheck %s --check-prefix=CLEANALL
+// CLEANALL: cc1
+// CLEANALL-SAME: -disable-free
+// CLEANALL-SAME: {{.*}}1.cpp
+// CLEANALL: cc1
+// CLEANALL-SAME: -disable-free
+// CLEANALL-SAME: {{.*}}2.cpp
+// CLEANALL: cc1
+// CLEANALL-SAME: -disable-free
+// CLEANALL-SAME: {{.*}}3.cpp
+
+// RUN: %clang -fno-integrated-cc1 %t1.cpp %t2.cpp %t3.cpp -### 2>&1 | FileCheck %s --check-prefix=CLEANALL
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -6143,7 +6143,7 @@
   if (Output.getType() == types::TY_Object &&
   Args.hasFlag(options::OPT__SLASH_showFilenames,
options::OPT__SLASH_showFilenames_, false)) {
-C.getJobs().getJobs().back()->setPrintInputFilenames(true);
+C.getJobs().getJobs().back()->PrintInputFilenames = true;
   }
 
   if (Arg *A = Args.getLastArg(options::OPT_pg))
Index: clang/lib/Driver/Job.cpp
===
--- clang/lib/Driver/Job.cpp
+++ clang/lib/Driver/Job.cpp
@@ -40,7 +40,7 @@
  const llvm::opt::ArgStringList ,
  ArrayRef Inputs)
 : Source(Source), Creator(Creator), Executable(Executable),
-  Arguments(Arguments) {
+  Arguments(Arguments), PrintInputFilenames(false), InProcess(false) {
   for (const auto  : Inputs)
 if (II.isFilename())
   InputFilenames.push_back(II.getFilename());
@@ -315,6 +315,15 @@
   Environment.push_back(nullptr);
 }
 
+// In some cases when running the calling process, we need to clean up if there
+// are other Commands being executed after us, to prevent bloating the heap.
+void Command::forceCleanUp() {
+  auto RemoveDisableFree = [](const char *A) {
+return StringRef(A).equals("-disable-free");
+  };
+  llvm::erase_if(Arguments, RemoveDisableFree);
+}
+
 void Command::PrintFileNames() const {
   if (PrintInputFilenames) {
 for (const char *Arg : InputFilenames)
@@ -371,6 +380,14 @@
/*memoryLimit*/ 0, ErrMsg, ExecutionFailed);
 }
 
+CC1Command::CC1Command(const Action , const Tool ,
+   const char *Executable,
+   const llvm::opt::ArgStringList ,
+   ArrayRef Inputs)
+: Command(Source, Creator, Executable, Arguments, Inputs) {
+  InProcess = true;
+}
+
 

[PATCH] D71775: [ThreadPool] On Windows, extend usage to all CPU sockets and all NUMA groups

[Reid Kleckner via Phabricator via cfe-commits]

rnk accepted this revision.
rnk added a comment.
This revision is now accepted and ready to land.
Herald added a reviewer: rriddle.
Herald added a subscriber: Joonsoo.
This revision now requires review to proceed.

lgtm




Comment at: llvm/lib/Support/Unix/Threading.inc:273
+
+int computeHostNumPhysicalThreads() {
+#if defined(HAVE_SCHED_GETAFFINITY) && defined(HAVE_CPU_COUNT)

aganea wrote:
> rnk wrote:
> > I'm not sure it makes sense to say "physical threads". I think "physical 
> > cores" was meant to distinguish between hardware threads and cores. 
> > Describing hardware execution resources as physical or non-physical isn't 
> > very precise or meaningful in the first place, but I don't think it should 
> > apply to hyper threads.
> `computeHostNumHardwareThreads()` ?
lgtm


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

https://reviews.llvm.org/D71775



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

[PATCH] D74455: [MS] Pass aligned, non-trivially copyable things indirectly on x86

[Reid Kleckner via Phabricator via cfe-commits]

rnk created this revision.
rnk added reviewers: craig.topper, erichkeane.
Herald added a project: clang.

This is a follow-up to case 1 in D72114 .

The C++ argument classification logic supersedes the C rules, so we have
to add a second check for overaligned records in the C++ argument
classification logic. Otherwise, clang will pass such records using
inalloca.

This has the interesting consequence of making it possible to opt a
record out of inalloca by adding a trivial alignment requirement
(alignas(void*, uint64_t)) to the record.

Fixes the last known corner case to PR44395


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D74455

Files:
  clang/lib/CodeGen/MicrosoftCXXABI.cpp
  clang/test/CodeGenCXX/inalloca-overaligned.cpp


Index: clang/test/CodeGenCXX/inalloca-overaligned.cpp
===
--- clang/test/CodeGenCXX/inalloca-overaligned.cpp
+++ clang/test/CodeGenCXX/inalloca-overaligned.cpp
@@ -4,10 +4,6 @@
 // MSVC passes overaligned types indirectly since MSVC 2015. Make sure that
 // works with inalloca.
 
-// FIXME: Pass non-trivial *and* overaligned types indirectly. Right now the 
C++
-// ABI rules say to use inalloca, and they take precedence, so it's not easy to
-// implement this.
-
 
 struct NonTrivial {
   NonTrivial();
@@ -50,3 +46,27 @@
 // CHECK: getelementptr inbounds <{ %struct.NonTrivial, %struct.OverAligned* 
}>, <{ %struct.NonTrivial, %struct.OverAligned* }>* %{{.*}}, i32 0, i32 1
 // CHECK: store %struct.OverAligned* [[TMP]], %struct.OverAligned** %{{.*}}, 
align 4
 // CHECK: call i32 @"?receive_inalloca_overaligned@@Y{{.*}}"(<{ 
%struct.NonTrivial, %struct.OverAligned* }>* inalloca %argmem)
+
+struct __declspec(align(64)) Both {
+  Both();
+  Both(const Both );
+  int x;
+};
+
+int receive_both1(Both b) {
+  return b.x;
+}
+// CHECK-LABEL: define dso_local i32 @"?receive_both1@@Y{{.*}}"
+// CHECK-SAME: (%struct.Both* %b)
+
+int receive_both2(Both x, Both y) {
+  return x.x + y.x;
+}
+// CHECK-LABEL: define dso_local i32 @"?receive_both2@@Y{{.*}}"
+// CHECK-SAME: (%struct.Both* %x, %struct.Both* %y)
+
+int receive_nt_and_both(NonTrivial x, Both y) {
+  return x.x + y.x;
+}
+// CHECK-LABEL: define dso_local i32 @"?receive_nt_and_both@@Y{{.*}}"
+// CHECK-SAME: (<{ %struct.NonTrivial, %struct.Both* }>* inalloca %0)
Index: clang/lib/CodeGen/MicrosoftCXXABI.cpp
===
--- clang/lib/CodeGen/MicrosoftCXXABI.cpp
+++ clang/lib/CodeGen/MicrosoftCXXABI.cpp
@@ -821,18 +821,19 @@
 return !RD->canPassInRegisters() ? RAA_Indirect : RAA_Default;
 
   case llvm::Triple::x86:
-// All record arguments are passed in memory on x86.  Decide whether to
-// construct the object directly in argument memory, or to construct the
-// argument elsewhere and copy the bytes during the call.
-
-// If C++ prohibits us from making a copy, construct the arguments directly
-// into argument memory.
-if (!RD->canPassInRegisters())
-  return RAA_DirectInMemory;
-
-// Otherwise, construct the argument into a temporary and copy the bytes
-// into the outgoing argument memory.
-return RAA_Default;
+// If C++ allows us to copy the struct bytes, use the normal C convention
+// rules.
+if (RD->canPassInRegisters())
+  return RAA_Default;
+
+// If the type has a required alignment (alignas, __declspec(align)), MSVC
+// passes it indirectly.
+if (getContext().isAlignmentRequired(getContext().getTypeDeclType(RD)))
+  return RAA_Indirect;
+
+// In the worst case, the argument must be constructed directly into the
+// outgoing argument memory slots.
+return RAA_DirectInMemory;
 
   case llvm::Triple::x86_64:
   case llvm::Triple::aarch64:


Index: clang/test/CodeGenCXX/inalloca-overaligned.cpp
===
--- clang/test/CodeGenCXX/inalloca-overaligned.cpp
+++ clang/test/CodeGenCXX/inalloca-overaligned.cpp
@@ -4,10 +4,6 @@
 // MSVC passes overaligned types indirectly since MSVC 2015. Make sure that
 // works with inalloca.
 
-// FIXME: Pass non-trivial *and* overaligned types indirectly. Right now the C++
-// ABI rules say to use inalloca, and they take precedence, so it's not easy to
-// implement this.
-
 
 struct NonTrivial {
   NonTrivial();
@@ -50,3 +46,27 @@
 // CHECK: getelementptr inbounds <{ %struct.NonTrivial, %struct.OverAligned* }>, <{ %struct.NonTrivial, %struct.OverAligned* }>* %{{.*}}, i32 0, i32 1
 // CHECK: store %struct.OverAligned* [[TMP]], %struct.OverAligned** %{{.*}}, align 4
 // CHECK: call i32 @"?receive_inalloca_overaligned@@Y{{.*}}"(<{ %struct.NonTrivial, %struct.OverAligned* }>* inalloca %argmem)
+
+struct __declspec(align(64)) Both {
+  Both();
+  Both(const Both );
+  int x;
+};
+
+int receive_both1(Both b) {
+  return b.x;
+}
+// CHECK-LABEL: define dso_local i32 @"?receive_both1@@Y{{.*}}"
+// 

[PATCH] D74372: [OpenMP][IRBuilder] Perform finalization (incl. outlining) late

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert added a comment.

I think this fixes the bug and allows loop optimizations later, let me know if 
there is anything else


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74372



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

[PATCH] D71830: [OpenMP][Part 2] Use reusable OpenMP context/traits handling

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert marked 4 inline comments as done.
jdoerfert added a comment.

@kiranchandramohan @JonChesterfield  I will address the comments just made, if 
you think this is otherwise fine it would be good if you accept the patch (it's 
been around for weeks so it's not really people didn't have a chance to look).




Comment at: clang/include/clang/Serialization/ASTRecordReader.h:272
+
+  template <> OpenMPTraitInfo *readUserType() { return readOpenMPTraitInfo(); }
+

kiranchandramohan wrote:
> jdoerfert wrote:
> > jdoerfert wrote:
> > > kiranchandramohan wrote:
> > > > jdoerfert wrote:
> > > > > kiranchandramohan wrote:
> > > > > > jdoerfert wrote:
> > > > > > > kiranchandramohan wrote:
> > > > > > > > Compiler throws up this error.
> > > > > > > > error: explicit specialization in non-namespace scope ‘class 
> > > > > > > > clang::ASTRecordReader’
> > > > > > > Oh, my compiler was happy. Let me rebase and see what the 
> > > > > > > pre-merge bots say so I might get some insight into the problem.
> > > > > > Were you able to reproduce the error? I was using gcc 9.2 compiler.
> > > > > I have not seen the error yet. I build with clang. Do you happen to 
> > > > > have an idea how to refactor this? I will look into it with a new gcc 
> > > > > asap.
> > > > Moving the specialization to the source file seems to fix the issue.
> > > I will do that then. Did you find the time to look over the rest?
> > > Moving the specialization to the source file seems to fix the issue.
> > 
> > Like this?
> You can remove the following declaration from the header file and just define 
> it in the cpp file. 
>  /// Specialization for OMPTraitInfo*.
>   template <> OMPTraitInfo *readUserType();
Right, will do that, thx!



Comment at: clang/lib/Serialization/ASTWriter.cpp:6575
 
+template <> void ASTRecordWriter::writeUserType(OMPTraitInfo *TI) {
+  writeUInt32(TI->Sets.size());

kiranchandramohan wrote:
> Had to also move this up in the file to avoid an instantiation after use 
> error.
I didn't get that but I have it in the header, I'll figure it out.



Comment at: llvm/lib/Frontend/OpenMP/OMPContext.cpp:417
+#include "llvm/Frontend/OpenMP/OMPKinds.def"
+  S.pop_back();
+  return S;

kiranchandramohan wrote:
> If there is no match? Or is it always guaranteed to have a match?
We always have selector sets (the outermost category). 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71830



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

[PATCH] D74452: [MS] Mark vectorcall FP and vector args inreg

[Reid Kleckner via Phabricator via cfe-commits]

rnk created this revision.
rnk added reviewers: craig.topper, erichkeane.
Herald added a project: clang.

This has no effect on how LLVM passes the arguments, but it prevents
rewriteWithInAlloca from thinking that these parameters should be part
of the inalloca pack.

Follow-up to D72114 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D74452

Files:
  clang/lib/CodeGen/TargetInfo.cpp
  clang/test/CodeGen/vectorcall.c
  clang/test/CodeGenCXX/inalloca-vector.cpp

Index: clang/test/CodeGenCXX/inalloca-vector.cpp
===
--- clang/test/CodeGenCXX/inalloca-vector.cpp
+++ clang/test/CodeGenCXX/inalloca-vector.cpp
@@ -66,14 +66,13 @@
  __m128 w, int edx, __m128 q, NonTrivial nt) {
   gv128 = x + y + z + w + q;
 }
-// FIXME: Enable these checks, clang generates wrong IR.
 // CHECK-LABEL: define dso_local x86_vectorcallcc void @"?vectorcall_receive_vec@@Y{{[^"]*}}"
-// CHECKX-SAME: (double inreg %xmm0,
-// CHECKX-SAME: double inreg %xmm1,
-// CHECKX-SAME: double inreg %xmm2,
-// CHECKX-SAME: <4 x float> inreg %x,
-// CHECKX-SAME: <4 x float> inreg %y,
-// CHECKX-SAME: <4 x float> inreg %z,
-// CHECKX-SAME: <4 x float>* inreg %0,
-// CHECKX-SAME: i32 inreg %edx,
-// CHECKX-SAME: <{ <4 x float>*, %struct.NonTrivial }>* inalloca %1)
+// CHECK-SAME: (double inreg %xmm0,
+// CHECK-SAME: double inreg %xmm1,
+// CHECK-SAME: double inreg %xmm2,
+// CHECK-SAME: <4 x float> inreg %x,
+// CHECK-SAME: <4 x float> inreg %y,
+// CHECK-SAME: <4 x float> inreg %z,
+// CHECK-SAME: <4 x float>* inreg %0,
+// CHECK-SAME: i32 inreg %edx,
+// CHECK-SAME: <{ <4 x float>*, %struct.NonTrivial }>* inalloca %1)
Index: clang/test/CodeGen/vectorcall.c
===
--- clang/test/CodeGen/vectorcall.c
+++ clang/test/CodeGen/vectorcall.c
@@ -31,13 +31,13 @@
 // indirectly. Additional vector arguments can consume the rest of the SSE
 // registers.
 void __vectorcall hfa2(struct HFA4 a, struct HFA4 b, double c) {}
-// X32: define dso_local x86_vectorcallcc void @"\01hfa2@@72"(%struct.HFA4 inreg %a.coerce, %struct.HFA4* inreg %b, double %c)
+// X32: define dso_local x86_vectorcallcc void @"\01hfa2@@72"(%struct.HFA4 inreg %a.coerce, %struct.HFA4* inreg %b, double inreg %c)
 // X64: define dso_local x86_vectorcallcc void @"\01hfa2@@72"(%struct.HFA4 inreg %a.coerce, %struct.HFA4* %b, double %c)
 
 // Ensure that we pass builtin types directly while counting them against the
 // SSE register usage.
 void __vectorcall hfa3(double a, double b, double c, double d, double e, struct HFA2 f) {}
-// X32: define dso_local x86_vectorcallcc void @"\01hfa3@@56"(double %a, double %b, double %c, double %d, double %e, %struct.HFA2* inreg %f)
+// X32: define dso_local x86_vectorcallcc void @"\01hfa3@@56"(double inreg %a, double inreg %b, double inreg %c, double inreg %d, double inreg %e, %struct.HFA2* inreg %f)
 // X64: define dso_local x86_vectorcallcc void @"\01hfa3@@56"(double %a, double %b, double %c, double %d, double %e, %struct.HFA2* %f)
 
 // Aggregates with more than four elements are not HFAs and are passed byval.
@@ -64,21 +64,21 @@
 // X64: define dso_local x86_vectorcallcc <4 x float> @"\01hva1@@80"(i32 %a, %struct.HVA4 inreg %b.coerce, i32 %c)
 
 v4f32 __vectorcall hva2(struct HVA4 a, struct HVA4 b, v4f32 c) {return c;}
-// X32: define dso_local x86_vectorcallcc <4 x float> @"\01hva2@@144"(%struct.HVA4 inreg %a.coerce, %struct.HVA4* inreg %b, <4 x float> %c)
+// X32: define dso_local x86_vectorcallcc <4 x float> @"\01hva2@@144"(%struct.HVA4 inreg %a.coerce, %struct.HVA4* inreg %b, <4 x float> inreg %c)
 // X64: define dso_local x86_vectorcallcc <4 x float> @"\01hva2@@144"(%struct.HVA4 inreg %a.coerce, %struct.HVA4* %b, <4 x float> %c)
 
 v4f32 __vectorcall hva3(v4f32 a, v4f32 b, v4f32 c, v4f32 d, v4f32 e, struct HVA2 f) {return f.x;}
-// X32: define dso_local x86_vectorcallcc <4 x float> @"\01hva3@@112"(<4 x float> %a, <4 x float> %b, <4 x float> %c, <4 x float> %d, <4 x float> %e, %struct.HVA2* inreg %f)
+// X32: define dso_local x86_vectorcallcc <4 x float> @"\01hva3@@112"(<4 x float> inreg %a, <4 x float> inreg %b, <4 x float> inreg %c, <4 x float> inreg %d, <4 x float> inreg %e, %struct.HVA2* inreg %f)
 // X64: define dso_local x86_vectorcallcc <4 x float> @"\01hva3@@112"(<4 x float> %a, <4 x float> %b, <4 x float> %c, <4 x float> %d, <4 x float> %e, %struct.HVA2* %f)
 
 // vector types have higher priority then HVA structures, So vector types are allocated first
 // and HVAs are allocated if enough registers are available
 v4f32 __vectorcall hva4(struct HVA4 a, struct HVA2 b, v4f32 c) {return b.y;}
-// X32: define dso_local x86_vectorcallcc <4 x float> @"\01hva4@@112"(%struct.HVA4 inreg %a.coerce, %struct.HVA2* inreg %b, <4 x float> %c)
+// X32: define dso_local x86_vectorcallcc <4 x float> @"\01hva4@@112"(%struct.HVA4 inreg %a.coerce, 

[PATCH] D71037: [Diagnostic] Add ftabstop to -Wmisleading-indentation

[Eli Friedman via Phabricator via cfe-commits]

efriedma added a comment.

I just ran into this warning, and I think there's a bit of a discoverability 
problem related to the width of tabs and -ftabstop.  If you have mixed tabs and 
spaces, and you've correctly specified the tab stop width with -ftabstop, 
everything works fine.  If you haven't specified -ftabstop, you get a warning, 
and the issue isn't obvious.  Depending on your editor settings, the code might 
look fine at first glance, and the warning text doesn't mention -ftabstop at 
all.  Maybe there's something that could be improved here, if we're printing a 
warning for two lines with mismatched indentation styles?


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

https://reviews.llvm.org/D71037



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

[clang] 2c6a389 - Re-land "[MS] Overhaul how clang passes overaligned args on x86_32"

[Reid Kleckner via cfe-commits]

Author: Reid Kleckner
Date: 2020-02-11T16:49:28-08:00
New Revision: 2c6a3896ab1de706389f27c921cf58084650f439

URL: 
https://github.com/llvm/llvm-project/commit/2c6a3896ab1de706389f27c921cf58084650f439
DIFF: 
https://github.com/llvm/llvm-project/commit/2c6a3896ab1de706389f27c921cf58084650f439.diff

LOG: Re-land "[MS] Overhaul how clang passes overaligned args on x86_32"

This brings back 2af74e27ed7d0832cbdde9cb969aaca7a42e99f9 and reverts
eaabaf7e04fe98990a8177a3e053346395efde1c.

The changes were correct, the code that was broken contained an ODR
violation that assumed that these types are passed equivalently:
  struct alignas(uint64_t) Wrapper { uint64_t P };
  void f(uint64_t p);
  void f(Wrapper p);

MSVC does not pass them the same way, and so clang-cl should not pass
them the same way either.

Added: 
clang/test/CodeGenCXX/inalloca-overaligned.cpp
clang/test/CodeGenCXX/inalloca-vector.cpp

Modified: 
clang/include/clang/CodeGen/CGFunctionInfo.h
clang/lib/CodeGen/CGCall.cpp
clang/lib/CodeGen/TargetInfo.cpp
clang/test/CodeGen/x86_32-arguments-win32.c

Removed: 




diff  --git a/clang/include/clang/CodeGen/CGFunctionInfo.h 
b/clang/include/clang/CodeGen/CGFunctionInfo.h
index 2a41ab9eece7..588c96afe402 100644
--- a/clang/include/clang/CodeGen/CGFunctionInfo.h
+++ b/clang/include/clang/CodeGen/CGFunctionInfo.h
@@ -88,6 +88,7 @@ class ABIArgInfo {
   Kind TheKind;
   bool PaddingInReg : 1;
   bool InAllocaSRet : 1;// isInAlloca()
+  bool InAllocaIndirect : 1;// isInAlloca()
   bool IndirectByVal : 1;   // isIndirect()
   bool IndirectRealign : 1; // isIndirect()
   bool SRetAfterThis : 1;   // isIndirect()
@@ -110,8 +111,8 @@ class ABIArgInfo {
 
 public:
   ABIArgInfo(Kind K = Direct)
-  : TypeData(nullptr), PaddingType(nullptr), DirectOffset(0),
-TheKind(K), PaddingInReg(false), InAllocaSRet(false),
+  : TypeData(nullptr), PaddingType(nullptr), DirectOffset(0), TheKind(K),
+PaddingInReg(false), InAllocaSRet(false), InAllocaIndirect(false),
 IndirectByVal(false), IndirectRealign(false), SRetAfterThis(false),
 InReg(false), CanBeFlattened(false), SignExt(false) {}
 
@@ -185,9 +186,10 @@ class ABIArgInfo {
 AI.setInReg(true);
 return AI;
   }
-  static ABIArgInfo getInAlloca(unsigned FieldIndex) {
+  static ABIArgInfo getInAlloca(unsigned FieldIndex, bool Indirect = false) {
 auto AI = ABIArgInfo(InAlloca);
 AI.setInAllocaFieldIndex(FieldIndex);
+AI.setInAllocaIndirect(Indirect);
 return AI;
   }
   static ABIArgInfo getExpand() {
@@ -380,6 +382,15 @@ class ABIArgInfo {
 AllocaFieldIndex = FieldIndex;
   }
 
+  unsigned getInAllocaIndirect() const {
+assert(isInAlloca() && "Invalid kind!");
+return InAllocaIndirect;
+  }
+  void setInAllocaIndirect(bool Indirect) {
+assert(isInAlloca() && "Invalid kind!");
+InAllocaIndirect = Indirect;
+  }
+
   /// Return true if this field of an inalloca struct should be returned
   /// to implement a struct return calling convention.
   bool getInAllocaSRet() const {

diff  --git a/clang/lib/CodeGen/CGCall.cpp b/clang/lib/CodeGen/CGCall.cpp
index e6cb4a1fd93f..c90326362d66 100644
--- a/clang/lib/CodeGen/CGCall.cpp
+++ b/clang/lib/CodeGen/CGCall.cpp
@@ -2370,6 +2370,9 @@ void CodeGenFunction::EmitFunctionProlog(const 
CGFunctionInfo ,
   auto FieldIndex = ArgI.getInAllocaFieldIndex();
   Address V =
   Builder.CreateStructGEP(ArgStruct, FieldIndex, Arg->getName());
+  if (ArgI.getInAllocaIndirect())
+V = Address(Builder.CreateLoad(V),
+getContext().getTypeAlignInChars(Ty));
   ArgVals.push_back(ParamValue::forIndirect(V));
   break;
 }
@@ -4091,18 +4094,39 @@ RValue CodeGenFunction::EmitCall(const CGFunctionInfo 
,
   assert(NumIRArgs == 0);
   assert(getTarget().getTriple().getArch() == llvm::Triple::x86);
   if (I->isAggregate()) {
-// Replace the placeholder with the appropriate argument slot GEP.
 Address Addr = I->hasLValue()
? I->getKnownLValue().getAddress(*this)
: I->getKnownRValue().getAggregateAddress();
 llvm::Instruction *Placeholder =
 cast(Addr.getPointer());
-CGBuilderTy::InsertPoint IP = Builder.saveIP();
-Builder.SetInsertPoint(Placeholder);
-Addr =
-Builder.CreateStructGEP(ArgMemory, 
ArgInfo.getInAllocaFieldIndex());
-Builder.restoreIP(IP);
+
+if (!ArgInfo.getInAllocaIndirect()) {
+  // Replace the placeholder with the appropriate argument slot GEP.
+  CGBuilderTy::InsertPoint IP = Builder.saveIP();
+  Builder.SetInsertPoint(Placeholder);
+  Addr = Builder.CreateStructGEP(ArgMemory,
+ ArgInfo.getInAllocaFieldIndex());
+  Builder.restoreIP(IP);
+} else {
+ 

[PATCH] D74447: [Clang] After integrated-cc1, ignore -disable-free when there are more than one job in the queue

[Reid Kleckner via Phabricator via cfe-commits]

rnk added inline comments.



Comment at: clang/include/clang/Driver/Job.h:59
   /// Whether to print the input filenames when executing.
   bool PrintInputFilenames = false;
 

Pack a new field after an existing bool for maximum micro optimization. :)



Comment at: clang/include/clang/Driver/Job.h:135
+  /// Whether the command will be executed in this process or not.
+  virtual bool inProcess() const { return false; }
+

I guess I lean towards making this a field, default initialized to false, set 
to true in the CC1Command constructor. Less relocations, etc.



Comment at: clang/lib/Driver/Driver.cpp:3759
+  if (!Jobs.empty()) {
+Command  = *llvm::reverse(Jobs).begin();
+for (auto  : Jobs) {

`*std::prev(Jobs.end())`? Or `*Jobs.getJobs().back()`, not quite sure.


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

https://reviews.llvm.org/D74447



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

[PATCH] D44609: [clang-format] New option BeforeLambdaBody to manage lambda line break inside function parameter call (in Allman style)

[Francois JEAN via Phabricator via cfe-commits]

Wawha added a comment.

Thank you @MyDeveloperDay for the validation! I'm happy that my contribution 
(and my work to do it well) is accepted.

And yes, I will be there to help to maintain that code and fix bugs if 
necessary.

About the permission in order to land that patch, I'm not sure that I will need 
to do. On that page, 
https://llvm.org/docs/DeveloperPolicy.html#new-contributors, I see that we can 
contact Chris to have permission on the repo. Is it what you propose? Or is 
there another way to have the permission only on llvm Phabricator?

Once I will know the right way to ask for that permission, I will do it :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D44609



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

[PATCH] D74447: [Clang] After integrated-cc1, ignore -disable-free when there are more than one job in the queue

[Alexandre Ganea via Phabricator via cfe-commits]

aganea updated this revision to Diff 244028.
aganea edited the summary of this revision.
aganea added a comment.

Wording.


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

https://reviews.llvm.org/D74447

Files:
  clang/include/clang/Driver/Job.h
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/Job.cpp
  clang/test/Driver/cc1-spawnprocess.c
  clang/tools/driver/cc1_main.cpp

Index: clang/tools/driver/cc1_main.cpp
===
--- clang/tools/driver/cc1_main.cpp
+++ clang/tools/driver/cc1_main.cpp
@@ -259,6 +259,7 @@
   // FIXME(ibiryukov): make profilerOutput flush in destructor instead.
   profilerOutput->flush();
   llvm::timeTraceProfilerCleanup();
+  Clang->clearOutputFiles(false);
 }
   }
 
Index: clang/test/Driver/cc1-spawnprocess.c
===
--- clang/test/Driver/cc1-spawnprocess.c
+++ clang/test/Driver/cc1-spawnprocess.c
@@ -20,3 +20,48 @@
 
 // YES: (in-process)
 // NO-NOT: (in-process)
+
+// The following tests ensure that only the last integrated-cc1 has -disable-free
+
+// RUN: echo 'int main() { return f() + g(); }' > %t1.cpp
+// RUN: echo 'int f() { return 1; }' > %t2.cpp
+// RUN: echo 'int g() { return 2; }' > %t3.cpp
+
+// Only one TU, one job, thus no cleanup is needed.
+// RUN: %clang -fintegrated-cc1 -c %t1.cpp -### 2>&1 | FileCheck %s --check-prefix=CLEAN
+// CLEAN: cc1
+// CLEAN-SAME: -disable-free
+
+// Three jobs, only the last invocation will skip clean up.
+// RUN: %clang -fintegrated-cc1 -c %t1.cpp %t2.cpp %t3.cpp -### 2>&1 | FileCheck %s --check-prefix=NOCLEAN-LAST
+// NOCLEAN-LAST: cc1
+// NOCLEAN-LAST-NOT: -disable-free
+// NOCLEAN-LAST-SAME: {{.*}}1.cpp
+// NOCLEAN-LAST: cc1
+// NOCLEAN-LAST-NOT: -disable-free
+// NOCLEAN-LAST-SAME: {{.*}}2.cpp
+// NOCLEAN-LAST: cc1
+// NOCLEAN-LAST-SAME: -disable-free
+// NOCLEAN-LAST-SAME: {{.*}}3.cpp
+
+// Four jobs, no invocation will skip clean up because the last job is linking.
+// RUN: %clang -fintegrated-cc1 %t1.cpp %t2.cpp %t3.cpp -### 2>&1 | FileCheck %s --check-prefix=ALL
+// ALL-NOT: -disable-free
+
+// In no-integrated-cc1 mode, no cleanup is needed, because we're running the
+// CC1 tool in a secondary process and the process' heap will be released
+// anyway when the process ends.
+// RUN: %clang -fno-integrated-cc1 -c %t1.cpp -### 2>&1 | FileCheck %s --check-prefix=CLEAN
+
+// RUN: %clang -fno-integrated-cc1 -c %t1.cpp %t2.cpp %t3.cpp -### 2>&1 | FileCheck %s --check-prefix=CLEANALL
+// CLEANALL: cc1
+// CLEANALL-SAME: -disable-free
+// CLEANALL-SAME: {{.*}}1.cpp
+// CLEANALL: cc1
+// CLEANALL-SAME: -disable-free
+// CLEANALL-SAME: {{.*}}2.cpp
+// CLEANALL: cc1
+// CLEANALL-SAME: -disable-free
+// CLEANALL-SAME: {{.*}}3.cpp
+
+// RUN: %clang -fno-integrated-cc1 %t1.cpp %t2.cpp %t3.cpp -### 2>&1 | FileCheck %s --check-prefix=CLEANALL
Index: clang/lib/Driver/Job.cpp
===
--- clang/lib/Driver/Job.cpp
+++ clang/lib/Driver/Job.cpp
@@ -315,6 +315,15 @@
   Environment.push_back(nullptr);
 }
 
+// In some cases when running the calling process, we need to clean up if there
+// are other Commands being executed after us, to prevent bloating the heap.
+void Command::forceCleanUp() {
+  auto RemoveDisableFree = [](const char *A) {
+return StringRef(A).equals("-disable-free");
+  };
+  llvm::erase_if(Arguments, RemoveDisableFree);
+}
+
 void Command::PrintFileNames() const {
   if (PrintInputFilenames) {
 for (const char *Arg : InputFilenames)
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -3753,6 +3753,18 @@
/*TargetDeviceOffloadKind*/ Action::OFK_None);
   }
 
+  // Make sure we clean up after in-process jobs.
+  JobList  = C.getJobs();
+  if (!Jobs.empty()) {
+Command  = *llvm::reverse(Jobs).begin();
+for (auto  : Jobs) {
+  // If this is a in-process job, and there are other jobs coming after,
+  // then make sure we clean up once the job is done.
+  if (Job.inProcess() &&  != )
+Job.forceCleanUp();
+}
+  }
+
   // If the user passed -Qunused-arguments or there were errors, don't warn
   // about any unused arguments.
   if (Diags.hasErrorOccurred() ||
Index: clang/include/clang/Driver/Job.h
===
--- clang/include/clang/Driver/Job.h
+++ clang/include/clang/Driver/Job.h
@@ -131,6 +131,12 @@
   /// Set whether to print the input filenames when executing.
   void setPrintInputFilenames(bool P) { PrintInputFilenames = P; }
 
+  /// Whether the command will be executed in this process or not.
+  virtual bool inProcess() const { return false; }
+
+  /// Prevent burying pointers, and ensure we free everything after execution.
+  void forceCleanUp();
+
 protected:
   /// Optionally 

[PATCH] D73742: [Clang][Driver] After default -fintegrated-cc1, fix report_fatal_error no longer generates preprocessed source + reproducer.sh

[Shoaib Meenai via Phabricator via cfe-commits]

smeenai added a comment.

In D73742#1871012 , @aganea wrote:

> In D73742#1870961 , @smeenai wrote:
>
> > I'm assuming this needs to be picked to 10.0?
>
>
> Yes! Is it up to the authors to integrate their patches to the release 
> branch? I'm seeing @hans is merging a lot of the patches.


I believe @hans takes care of the merging; I just wanted to make sure this 
hadn't been missed :) I'm gonna assume this is on his radar.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73742



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

[PATCH] D73742: [Clang][Driver] After default -fintegrated-cc1, fix report_fatal_error no longer generates preprocessed source + reproducer.sh

[Alexandre Ganea via Phabricator via cfe-commits]

aganea marked an inline comment as done.
aganea added a comment.

In D73742#1870961 , @smeenai wrote:

> I'm assuming this needs to be picked to 10.0?


Yes! Is it up to the authors to integrate their patches to the release branch? 
I'm seeing @hans is merging a lot of the patches.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73742



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

[PATCH] D71830: [OpenMP][Part 2] Use reusable OpenMP context/traits handling

[Kiran Chandramohan via Phabricator via cfe-commits]

kiranchandramohan added inline comments.



Comment at: clang/lib/Serialization/ASTWriter.cpp:6575
 
+template <> void ASTRecordWriter::writeUserType(OMPTraitInfo *TI) {
+  writeUInt32(TI->Sets.size());

Had to also move this up in the file to avoid an instantiation after use error.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71830



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

[PATCH] D73742: [Clang][Driver] After default -fintegrated-cc1, fix report_fatal_error no longer generates preprocessed source + reproducer.sh

[Shoaib Meenai via Phabricator via cfe-commits]

smeenai added a comment.

I'm assuming this needs to be picked to 10.0?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73742



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

[PATCH] D71830: [OpenMP][Part 2] Use reusable OpenMP context/traits handling

[Kiran Chandramohan via Phabricator via cfe-commits]

kiranchandramohan added a comment.

Looks OK to me. 
Couple of comments inline.




Comment at: clang/include/clang/Serialization/ASTRecordReader.h:272
+
+  template <> OpenMPTraitInfo *readUserType() { return readOpenMPTraitInfo(); }
+

jdoerfert wrote:
> jdoerfert wrote:
> > kiranchandramohan wrote:
> > > jdoerfert wrote:
> > > > kiranchandramohan wrote:
> > > > > jdoerfert wrote:
> > > > > > kiranchandramohan wrote:
> > > > > > > Compiler throws up this error.
> > > > > > > error: explicit specialization in non-namespace scope ‘class 
> > > > > > > clang::ASTRecordReader’
> > > > > > Oh, my compiler was happy. Let me rebase and see what the pre-merge 
> > > > > > bots say so I might get some insight into the problem.
> > > > > Were you able to reproduce the error? I was using gcc 9.2 compiler.
> > > > I have not seen the error yet. I build with clang. Do you happen to 
> > > > have an idea how to refactor this? I will look into it with a new gcc 
> > > > asap.
> > > Moving the specialization to the source file seems to fix the issue.
> > I will do that then. Did you find the time to look over the rest?
> > Moving the specialization to the source file seems to fix the issue.
> 
> Like this?
You can remove the following declaration from the header file and just define 
it in the cpp file. 
 /// Specialization for OMPTraitInfo*.
  template <> OMPTraitInfo *readUserType();



Comment at: llvm/lib/Frontend/OpenMP/OMPContext.cpp:417
+#include "llvm/Frontend/OpenMP/OMPKinds.def"
+  S.pop_back();
+  return S;

If there is no match? Or is it always guaranteed to have a match?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71830



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

[PATCH] D74423: Use C++14-style return type deduction in clang.

[Justin Lebar via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGac66c61bf946: Use C++14-style return type deduction in 
clang. (authored by jlebar).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74423

Files:
  clang/lib/AST/ASTImporter.cpp
  clang/lib/StaticAnalyzer/Checkers/GCDAntipatternChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/OSObjectCStyleCast.cpp
  clang/lib/StaticAnalyzer/Checkers/ObjCAutoreleaseWriteChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/PointerSortingChecker.cpp


Index: clang/lib/StaticAnalyzer/Checkers/PointerSortingChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/PointerSortingChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/PointerSortingChecker.cpp
@@ -54,7 +54,7 @@
  OS.str(), Location, Range);
 }
 
-auto callsName(const char *FunctionName) -> decltype(callee(functionDecl())) {
+decltype(auto) callsName(const char *FunctionName) {
   return callee(functionDecl(hasName(FunctionName)));
 }
 
Index: clang/lib/StaticAnalyzer/Checkers/ObjCAutoreleaseWriteChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/ObjCAutoreleaseWriteChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/ObjCAutoreleaseWriteChecker.cpp
@@ -100,8 +100,7 @@
   return std::vector(V.begin(), V.end());
 }
 
-static auto callsNames(std::vector FunctionNames)
--> decltype(callee(functionDecl())) {
+static decltype(auto) callsNames(std::vector FunctionNames) {
   return callee(functionDecl(hasAnyName(toRefs(FunctionNames;
 }
 
Index: clang/lib/StaticAnalyzer/Checkers/OSObjectCStyleCast.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/OSObjectCStyleCast.cpp
+++ clang/lib/StaticAnalyzer/Checkers/OSObjectCStyleCast.cpp
@@ -55,8 +55,7 @@
 CE->getSourceRange());
 }
 
-static auto hasTypePointingTo(DeclarationMatcher DeclM)
--> decltype(hasType(pointerType())) {
+static decltype(auto) hasTypePointingTo(DeclarationMatcher DeclM) {
   return hasType(pointerType(pointee(hasDeclaration(DeclM;
 }
 
Index: clang/lib/StaticAnalyzer/Checkers/GCDAntipatternChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/GCDAntipatternChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/GCDAntipatternChecker.cpp
@@ -52,18 +52,16 @@
 BugReporter ) const;
 };
 
-auto callsName(const char *FunctionName)
--> decltype(callee(functionDecl())) {
+decltype(auto) callsName(const char *FunctionName) {
   return callee(functionDecl(hasName(FunctionName)));
 }
 
-auto equalsBoundArgDecl(int ArgIdx, const char *DeclName)
--> decltype(hasArgument(0, expr())) {
+decltype(auto) equalsBoundArgDecl(int ArgIdx, const char *DeclName) {
   return hasArgument(ArgIdx, ignoringParenCasts(declRefExpr(
  to(varDecl(equalsBoundNode(DeclName));
 }
 
-auto bindAssignmentToDecl(const char *DeclName) -> decltype(hasLHS(expr())) {
+decltype(auto) bindAssignmentToDecl(const char *DeclName) {
   return hasLHS(ignoringParenImpCasts(
  declRefExpr(to(varDecl().bind(DeclName);
 }
Index: clang/lib/AST/ASTImporter.cpp
===
--- clang/lib/AST/ASTImporter.cpp
+++ clang/lib/AST/ASTImporter.cpp
@@ -204,9 +204,7 @@
 
 // Wrapper for an overload set.
 template  struct CallOverloadedCreateFun {
-  template 
-  auto operator()(Args &&... args)
-  -> decltype(ToDeclT::Create(std::forward(args)...)) {
+  template  decltype(auto) operator()(Args &&... args) {
 return ToDeclT::Create(std::forward(args)...);
   }
 };


Index: clang/lib/StaticAnalyzer/Checkers/PointerSortingChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/PointerSortingChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/PointerSortingChecker.cpp
@@ -54,7 +54,7 @@
  OS.str(), Location, Range);
 }
 
-auto callsName(const char *FunctionName) -> decltype(callee(functionDecl())) {
+decltype(auto) callsName(const char *FunctionName) {
   return callee(functionDecl(hasName(FunctionName)));
 }
 
Index: clang/lib/StaticAnalyzer/Checkers/ObjCAutoreleaseWriteChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/ObjCAutoreleaseWriteChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/ObjCAutoreleaseWriteChecker.cpp
@@ -100,8 +100,7 @@
   return std::vector(V.begin(), V.end());
 }
 
-static auto callsNames(std::vector FunctionNames)
--> decltype(callee(functionDecl())) {
+static decltype(auto) callsNames(std::vector FunctionNames) {
   return 

[clang] ac66c61 - Use C++14-style return type deduction in clang.

[Justin Lebar via cfe-commits]

Author: Justin Lebar
Date: 2020-02-11T14:41:22-08:00
New Revision: ac66c61bf9463bf419102ad8b6565dcbc495b0ab

URL: 
https://github.com/llvm/llvm-project/commit/ac66c61bf9463bf419102ad8b6565dcbc495b0ab
DIFF: 
https://github.com/llvm/llvm-project/commit/ac66c61bf9463bf419102ad8b6565dcbc495b0ab.diff

LOG: Use C++14-style return type deduction in clang.

Summary:
Simplifies the C++11-style "-> decltype(...)" return-type deduction.

Note that you have to be careful about whether the function return type
is `auto` or `decltype(auto)`.  The difference is that bare `auto`
strips const and reference, just like lambda return type deduction.  In
some cases that's what we want (or more likely, we know that the return
type is a value type), but whenever we're wrapping a templated function
which might return a reference, we need to be sure that the return type
is decltype(auto).

No functional change.

Reviewers: bkramer, MaskRay, martong, shafik

Subscribers: martong, cfe-commits

Tags: #clang

Differential Revision: https://reviews.llvm.org/D74423

Added: 


Modified: 
clang/lib/AST/ASTImporter.cpp
clang/lib/StaticAnalyzer/Checkers/GCDAntipatternChecker.cpp
clang/lib/StaticAnalyzer/Checkers/OSObjectCStyleCast.cpp
clang/lib/StaticAnalyzer/Checkers/ObjCAutoreleaseWriteChecker.cpp
clang/lib/StaticAnalyzer/Checkers/PointerSortingChecker.cpp

Removed: 




diff  --git a/clang/lib/AST/ASTImporter.cpp b/clang/lib/AST/ASTImporter.cpp
index 3361c7886c2d..49058ceaab25 100644
--- a/clang/lib/AST/ASTImporter.cpp
+++ b/clang/lib/AST/ASTImporter.cpp
@@ -204,9 +204,7 @@ namespace clang {
 
 // Wrapper for an overload set.
 template  struct CallOverloadedCreateFun {
-  template 
-  auto operator()(Args &&... args)
-  -> decltype(ToDeclT::Create(std::forward(args)...)) {
+  template  decltype(auto) operator()(Args &&... args) {
 return ToDeclT::Create(std::forward(args)...);
   }
 };

diff  --git a/clang/lib/StaticAnalyzer/Checkers/GCDAntipatternChecker.cpp 
b/clang/lib/StaticAnalyzer/Checkers/GCDAntipatternChecker.cpp
index d471c23b83bf..76fa56406443 100644
--- a/clang/lib/StaticAnalyzer/Checkers/GCDAntipatternChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/GCDAntipatternChecker.cpp
@@ -52,18 +52,16 @@ class GCDAntipatternChecker : public 
Checker {
 BugReporter ) const;
 };
 
-auto callsName(const char *FunctionName)
--> decltype(callee(functionDecl())) {
+decltype(auto) callsName(const char *FunctionName) {
   return callee(functionDecl(hasName(FunctionName)));
 }
 
-auto equalsBoundArgDecl(int ArgIdx, const char *DeclName)
--> decltype(hasArgument(0, expr())) {
+decltype(auto) equalsBoundArgDecl(int ArgIdx, const char *DeclName) {
   return hasArgument(ArgIdx, ignoringParenCasts(declRefExpr(
  to(varDecl(equalsBoundNode(DeclName));
 }
 
-auto bindAssignmentToDecl(const char *DeclName) -> decltype(hasLHS(expr())) {
+decltype(auto) bindAssignmentToDecl(const char *DeclName) {
   return hasLHS(ignoringParenImpCasts(
  declRefExpr(to(varDecl().bind(DeclName);
 }

diff  --git a/clang/lib/StaticAnalyzer/Checkers/OSObjectCStyleCast.cpp 
b/clang/lib/StaticAnalyzer/Checkers/OSObjectCStyleCast.cpp
index 5b9895c338d8..90d69b81305c 100644
--- a/clang/lib/StaticAnalyzer/Checkers/OSObjectCStyleCast.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/OSObjectCStyleCast.cpp
@@ -55,8 +55,7 @@ static void emitDiagnostics(const BoundNodes ,
 CE->getSourceRange());
 }
 
-static auto hasTypePointingTo(DeclarationMatcher DeclM)
--> decltype(hasType(pointerType())) {
+static decltype(auto) hasTypePointingTo(DeclarationMatcher DeclM) {
   return hasType(pointerType(pointee(hasDeclaration(DeclM;
 }
 

diff  --git a/clang/lib/StaticAnalyzer/Checkers/ObjCAutoreleaseWriteChecker.cpp 
b/clang/lib/StaticAnalyzer/Checkers/ObjCAutoreleaseWriteChecker.cpp
index d2371fe60d21..9d587c585650 100644
--- a/clang/lib/StaticAnalyzer/Checkers/ObjCAutoreleaseWriteChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/ObjCAutoreleaseWriteChecker.cpp
@@ -100,8 +100,7 @@ static inline std::vector 
toRefs(std::vector V) {
   return std::vector(V.begin(), V.end());
 }
 
-static auto callsNames(std::vector FunctionNames)
--> decltype(callee(functionDecl())) {
+static decltype(auto) callsNames(std::vector FunctionNames) {
   return callee(functionDecl(hasAnyName(toRefs(FunctionNames;
 }
 

diff  --git a/clang/lib/StaticAnalyzer/Checkers/PointerSortingChecker.cpp 
b/clang/lib/StaticAnalyzer/Checkers/PointerSortingChecker.cpp
index 586d9d3af2a6..a3bfac97e40a 100644
--- a/clang/lib/StaticAnalyzer/Checkers/PointerSortingChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/PointerSortingChecker.cpp
@@ -54,7 +54,7 @@ static void emitDiagnostics(const BoundNodes , const 
Decl *D,
  OS.str(), Location, Range);
 

[PATCH] D71830: [OpenMP][Part 2] Use reusable OpenMP context/traits handling

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert added a comment.

@kiranchandramohan Do you have more input on this one?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71830



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

[PATCH] D74447: [Clang] After integrated-cc1, ignore -disable-free when there are more than one job in the queue

[Alexandre Ganea via Phabricator via cfe-commits]

aganea created this revision.
aganea added reviewers: hans, thakis, rnk, erichkeane.
Herald added subscribers: cfe-commits, dexonsmith.
Herald added a project: clang.

A `CC1Command` was previously always burying pointers (ie. skipping object 
deletion). This lead to higher memory usage scenarios, when the inputs or 
cmd-line flags expand to more than one driver job.

With this patch, when several jobs are created by the driver, and at least one 
`CC1Command` is in the job queue, remove `-disable-free` for `CC1Command`s 
other than the last one.
If the last job is not a `CC1Command`, this patch disables `-disable-free` 
altogether for `CC1Command`s.

Fixes PR44823.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D74447

Files:
  clang/include/clang/Driver/Job.h
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/Job.cpp
  clang/test/Driver/cc1-spawnprocess.c
  clang/tools/driver/cc1_main.cpp

Index: clang/tools/driver/cc1_main.cpp
===
--- clang/tools/driver/cc1_main.cpp
+++ clang/tools/driver/cc1_main.cpp
@@ -259,6 +259,7 @@
   // FIXME(ibiryukov): make profilerOutput flush in destructor instead.
   profilerOutput->flush();
   llvm::timeTraceProfilerCleanup();
+  Clang->clearOutputFiles(false);
 }
   }
 
Index: clang/test/Driver/cc1-spawnprocess.c
===
--- clang/test/Driver/cc1-spawnprocess.c
+++ clang/test/Driver/cc1-spawnprocess.c
@@ -20,3 +20,48 @@
 
 // YES: (in-process)
 // NO-NOT: (in-process)
+
+// The following tests ensure that only the last integrated-cc1 has -disable-free
+
+// RUN: echo 'int main() { return f() + g(); }' > %t1.cpp
+// RUN: echo 'int f() { return 1; }' > %t2.cpp
+// RUN: echo 'int g() { return 2; }' > %t3.cpp
+
+// Only one TU, one job, thus no cleanup is needed.
+// RUN: %clang -fintegrated-cc1 -c %t1.cpp -### 2>&1 | FileCheck %s --check-prefix=CLEAN
+// CLEAN: cc1
+// CLEAN-SAME: -disable-free
+
+// Three jobs, only the last invocation will skip clean up.
+// RUN: %clang -fintegrated-cc1 -c %t1.cpp %t2.cpp %t3.cpp -### 2>&1 | FileCheck %s --check-prefix=NOCLEAN-LAST
+// NOCLEAN-LAST: cc1
+// NOCLEAN-LAST-NOT: -disable-free
+// NOCLEAN-LAST-SAME: {{.*}}1.cpp
+// NOCLEAN-LAST: cc1
+// NOCLEAN-LAST-NOT: -disable-free
+// NOCLEAN-LAST-SAME: {{.*}}2.cpp
+// NOCLEAN-LAST: cc1
+// NOCLEAN-LAST-SAME: -disable-free
+// NOCLEAN-LAST-SAME: {{.*}}3.cpp
+
+// Four jobs, no invocation will skip clean up because the last job is linking.
+// RUN: %clang -fintegrated-cc1 %t1.cpp %t2.cpp %t3.cpp -### 2>&1 | FileCheck %s --check-prefix=ALL
+// ALL-NOT: -disable-free
+
+// In no-integrated-cc1 mode, no cleanup is needed, because we're running the
+// CC1 tool in a secondary process and the process' heap will be released
+// anyway when the process ends.
+// RUN: %clang -fno-integrated-cc1 -c %t1.cpp -### 2>&1 | FileCheck %s --check-prefix=CLEAN
+
+// RUN: %clang -fno-integrated-cc1 -c %t1.cpp %t2.cpp %t3.cpp -### 2>&1 | FileCheck %s --check-prefix=CLEANALL
+// CLEANALL: cc1
+// CLEANALL-SAME: -disable-free
+// CLEANALL-SAME: {{.*}}1.cpp
+// CLEANALL: cc1
+// CLEANALL-SAME: -disable-free
+// CLEANALL-SAME: {{.*}}2.cpp
+// CLEANALL: cc1
+// CLEANALL-SAME: -disable-free
+// CLEANALL-SAME: {{.*}}3.cpp
+
+// RUN: %clang -fno-integrated-cc1 %t1.cpp %t2.cpp %t3.cpp -### 2>&1 | FileCheck %s --check-prefix=CLEANALL
Index: clang/lib/Driver/Job.cpp
===
--- clang/lib/Driver/Job.cpp
+++ clang/lib/Driver/Job.cpp
@@ -315,6 +315,15 @@
   Environment.push_back(nullptr);
 }
 
+// In some cases when running the calling process, we need to clean-up if there
+// are other Commands being executed after us, to prevent bloating the heap.
+void Command::forceClean() {
+  auto RemoveDisableFree = [](const char *A) {
+return StringRef(A).equals("-disable-free");
+  };
+  llvm::erase_if(Arguments, RemoveDisableFree);
+}
+
 void Command::PrintFileNames() const {
   if (PrintInputFilenames) {
 for (const char *Arg : InputFilenames)
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -3753,6 +3753,18 @@
/*TargetDeviceOffloadKind*/ Action::OFK_None);
   }
 
+  // Make sure we clean-up after in-process jobs.
+  JobList  = C.getJobs();
+  if (!Jobs.empty()) {
+Command  = *llvm::reverse(Jobs).begin();
+for (auto  : Jobs) {
+  // If this is a in-process job, and there are other jobs coming after,
+  // then make sure we clean up once the job is done.
+  if (Job.inProcess() &&  != )
+Job.forceClean();
+}
+  }
+
   // If the user passed -Qunused-arguments or there were errors, don't warn
   // about any unused arguments.
   if (Diags.hasErrorOccurred() ||
Index: clang/include/clang/Driver/Job.h

[PATCH] D74372: [OpenMP][IRBuilder] Perform finalization (incl. outlining) late

[Vedant Kumar via Phabricator via cfe-commits]

vsk added inline comments.



Comment at: llvm/lib/Transforms/Utils/CodeExtractor.cpp:1408
 
-  // See llvm.org/PR44560, OpenMP passes an invalid subprogram to 
CodeExtractor.
-  bool NeedWorkaroundForOpenMPIRBuilderBug =
-  OldSP && OldSP->getRetainedNodes()->isTemporary();
-
-  if (!OldSP || NeedWorkaroundForOpenMPIRBuilderBug) {
+  if (!OldSP) {
 // Erase any debug info the new function contains.

This part lgtm, thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74372



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

[PATCH] D74372: [OpenMP][IRBuilder] Perform finalization (incl. outlining) late

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert updated this revision to Diff 244005.
jdoerfert added a comment.

Improve comment


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74372

Files:
  clang/lib/CodeGen/CodeGenFunction.cpp
  llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
  llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
  llvm/lib/Transforms/Utils/CodeExtractor.cpp
  llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp

Index: llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp
===
--- llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp
+++ llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp
@@ -96,7 +96,7 @@
   EXPECT_EQ(cast(Barrier)->getArgOperand(1), GTID);
 
   Builder.CreateUnreachable();
-  EXPECT_FALSE(verifyModule(*M));
+  EXPECT_FALSE(verifyModule(*M, ()));
 }
 
 TEST_F(OpenMPIRBuilderTest, CreateCancel) {
@@ -151,7 +151,7 @@
   OMPBuilder.popFinalizationCB();
 
   Builder.CreateUnreachable();
-  EXPECT_FALSE(verifyModule(*M));
+  EXPECT_FALSE(verifyModule(*M, ()));
 }
 
 TEST_F(OpenMPIRBuilderTest, CreateCancelIfCond) {
@@ -212,7 +212,7 @@
   OMPBuilder.popFinalizationCB();
 
   Builder.CreateUnreachable();
-  EXPECT_FALSE(verifyModule(*M));
+  EXPECT_FALSE(verifyModule(*M, ()));
 }
 
 TEST_F(OpenMPIRBuilderTest, CreateCancelBarrier) {
@@ -267,7 +267,7 @@
   OMPBuilder.popFinalizationCB();
 
   Builder.CreateUnreachable();
-  EXPECT_FALSE(verifyModule(*M));
+  EXPECT_FALSE(verifyModule(*M, ()));
 }
 
 TEST_F(OpenMPIRBuilderTest, DbgLoc) {
@@ -362,9 +362,9 @@
 
   auto FiniCB = [&](InsertPointTy CodeGenIP) { ++NumFinalizationPoints; };
 
-  IRBuilder<>::InsertPoint AfterIP = OMPBuilder.CreateParallel(
-  Loc, BodyGenCB, PrivCB, FiniCB, nullptr, nullptr, OMP_PROC_BIND_default, false);
-
+  IRBuilder<>::InsertPoint AfterIP =
+  OMPBuilder.CreateParallel(Loc, BodyGenCB, PrivCB, FiniCB, nullptr,
+nullptr, OMP_PROC_BIND_default, false);
   EXPECT_EQ(NumBodiesGenerated, 1U);
   EXPECT_EQ(NumPrivatizedVars, 1U);
   EXPECT_EQ(NumFinalizationPoints, 1U);
@@ -372,10 +372,12 @@
   Builder.restoreIP(AfterIP);
   Builder.CreateRetVoid();
 
+  OMPBuilder.finalize();
+
   EXPECT_NE(PrivAI, nullptr);
   Function *OutlinedFn = PrivAI->getFunction();
   EXPECT_NE(F, OutlinedFn);
-  EXPECT_FALSE(verifyModule(*M));
+  EXPECT_FALSE(verifyModule(*M, ()));
   EXPECT_TRUE(OutlinedFn->hasFnAttribute(Attribute::NoUnwind));
   EXPECT_TRUE(OutlinedFn->hasFnAttribute(Attribute::NoRecurse));
   EXPECT_TRUE(OutlinedFn->hasParamAttribute(0, Attribute::NoAlias));
@@ -470,6 +472,7 @@
 
   Builder.restoreIP(AfterIP);
   Builder.CreateRetVoid();
+  OMPBuilder.finalize();
 
   EXPECT_NE(PrivAI, nullptr);
   Function *OutlinedFn = PrivAI->getFunction();
@@ -595,6 +598,7 @@
 
   Builder.restoreIP(AfterIP);
   Builder.CreateRetVoid();
+  OMPBuilder.finalize();
 
   EXPECT_FALSE(verifyModule(*M, ()));
 
Index: llvm/lib/Transforms/Utils/CodeExtractor.cpp
===
--- llvm/lib/Transforms/Utils/CodeExtractor.cpp
+++ llvm/lib/Transforms/Utils/CodeExtractor.cpp
@@ -1405,11 +1405,7 @@
   DISubprogram *OldSP = OldFunc.getSubprogram();
   LLVMContext  = OldFunc.getContext();
 
-  // See llvm.org/PR44560, OpenMP passes an invalid subprogram to CodeExtractor.
-  bool NeedWorkaroundForOpenMPIRBuilderBug =
-  OldSP && OldSP->getRetainedNodes()->isTemporary();
-
-  if (!OldSP || NeedWorkaroundForOpenMPIRBuilderBug) {
+  if (!OldSP) {
 // Erase any debug info the new function contains.
 stripDebugInfo(NewFunc);
 // Make sure the old function doesn't contain any non-local metadata refs.
Index: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
===
--- llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
+++ llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
@@ -93,6 +93,55 @@
 
 void OpenMPIRBuilder::initialize() { initializeTypes(M); }
 
+void OpenMPIRBuilder::finalize() {
+  for (OutlineInfo  : OutlineInfos) {
+assert(!OI.Blocks.empty() &&
+   "Outlined regions should have at least a single block!");
+BasicBlock *RegEntryBB = OI.Blocks.front();
+Function *OuterFn = RegEntryBB->getParent();
+CodeExtractorAnalysisCache CEAC(*OuterFn);
+CodeExtractor Extractor(OI.Blocks, /* DominatorTree */ nullptr,
+/* AggregateArgs */ false,
+/* BlockFrequencyInfo */ nullptr,
+/* BranchProbabilityInfo */ nullptr,
+/* AssumptionCache */ nullptr,
+/* AllowVarArgs */ true,
+/* AllowAlloca */ true,
+/* Suffix */ ".omp_par");
+
+LLVM_DEBUG(dbgs() << "Before outlining: " << *OuterFn << "\n");
+
+Function *OutlinedFn = Extractor.extractCodeRegion(CEAC);
+
+LLVM_DEBUG(dbgs() << "After  

[PATCH] D74386: [SVE] Update API ConstantVector::getSplat() to use ElementCount.

[Eli Friedman via Phabricator via cfe-commits]

efriedma added a comment.

This is sort of a large number of functional changes, but it's probably okay to 
skip test coverage for all the simple getNumElements->getElementCount changes; 
it's obvious it can't break anything.




Comment at: llvm/include/llvm/Analysis/Utils/Local.h:67
+OpC = ConstantVector::getSplat(
+cast(IntIdxTy)->getElementCount(), OpC);
 

getVectorElementCount?  (Same in other places.)



Comment at: llvm/lib/IR/Constants.cpp:2129
+  if (EltCount.Min != 0)
+ReqTy = VectorType::get(ReqTy, EltCount);
 

I'd like to see test coverage for the GEP changes here, since the logic is 
relatively intricate.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74386



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

[PATCH] D74372: [OpenMP][IRBuilder] Perform finalization (incl. outlining) late

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert added a comment.

In D74372#1870620 , @vsk wrote:

> Thanks :). IIRC `check-clang` is enough to exercise the relevant code path, 
> as we get an assert in CodeExtractor without the workaround. (side note: 
> please don't take my comments here as blocking, I just wanted to see if we 
> could delete some cruft)


Can you verify I deleted the workaround? I was not sure if the workaround is 
all of it or only the OMP... part.
I run the test you mentioned w/ and w/o the debug flag you mentioned in the PR, 
both times it passes w/o assertion:

  PASS: Clang :: OpenMP/parallel_codegen.cpp 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74372



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

[PATCH] D74372: [OpenMP][IRBuilder] Perform finalization (incl. outlining) late

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert updated this revision to Diff 244002.
jdoerfert marked 6 inline comments as done.
jdoerfert added a comment.

Removed OMPIRBuilder workaround in extractor and addressed comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74372

Files:
  clang/lib/CodeGen/CodeGenFunction.cpp
  llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
  llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
  llvm/lib/Transforms/Utils/CodeExtractor.cpp
  llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp

Index: llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp
===
--- llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp
+++ llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp
@@ -96,7 +96,7 @@
   EXPECT_EQ(cast(Barrier)->getArgOperand(1), GTID);
 
   Builder.CreateUnreachable();
-  EXPECT_FALSE(verifyModule(*M));
+  EXPECT_FALSE(verifyModule(*M, ()));
 }
 
 TEST_F(OpenMPIRBuilderTest, CreateCancel) {
@@ -151,7 +151,7 @@
   OMPBuilder.popFinalizationCB();
 
   Builder.CreateUnreachable();
-  EXPECT_FALSE(verifyModule(*M));
+  EXPECT_FALSE(verifyModule(*M, ()));
 }
 
 TEST_F(OpenMPIRBuilderTest, CreateCancelIfCond) {
@@ -212,7 +212,7 @@
   OMPBuilder.popFinalizationCB();
 
   Builder.CreateUnreachable();
-  EXPECT_FALSE(verifyModule(*M));
+  EXPECT_FALSE(verifyModule(*M, ()));
 }
 
 TEST_F(OpenMPIRBuilderTest, CreateCancelBarrier) {
@@ -267,7 +267,7 @@
   OMPBuilder.popFinalizationCB();
 
   Builder.CreateUnreachable();
-  EXPECT_FALSE(verifyModule(*M));
+  EXPECT_FALSE(verifyModule(*M, ()));
 }
 
 TEST_F(OpenMPIRBuilderTest, DbgLoc) {
@@ -362,9 +362,9 @@
 
   auto FiniCB = [&](InsertPointTy CodeGenIP) { ++NumFinalizationPoints; };
 
-  IRBuilder<>::InsertPoint AfterIP = OMPBuilder.CreateParallel(
-  Loc, BodyGenCB, PrivCB, FiniCB, nullptr, nullptr, OMP_PROC_BIND_default, false);
-
+  IRBuilder<>::InsertPoint AfterIP =
+  OMPBuilder.CreateParallel(Loc, BodyGenCB, PrivCB, FiniCB, nullptr,
+nullptr, OMP_PROC_BIND_default, false);
   EXPECT_EQ(NumBodiesGenerated, 1U);
   EXPECT_EQ(NumPrivatizedVars, 1U);
   EXPECT_EQ(NumFinalizationPoints, 1U);
@@ -372,10 +372,12 @@
   Builder.restoreIP(AfterIP);
   Builder.CreateRetVoid();
 
+  OMPBuilder.finalize();
+
   EXPECT_NE(PrivAI, nullptr);
   Function *OutlinedFn = PrivAI->getFunction();
   EXPECT_NE(F, OutlinedFn);
-  EXPECT_FALSE(verifyModule(*M));
+  EXPECT_FALSE(verifyModule(*M, ()));
   EXPECT_TRUE(OutlinedFn->hasFnAttribute(Attribute::NoUnwind));
   EXPECT_TRUE(OutlinedFn->hasFnAttribute(Attribute::NoRecurse));
   EXPECT_TRUE(OutlinedFn->hasParamAttribute(0, Attribute::NoAlias));
@@ -470,6 +472,7 @@
 
   Builder.restoreIP(AfterIP);
   Builder.CreateRetVoid();
+  OMPBuilder.finalize();
 
   EXPECT_NE(PrivAI, nullptr);
   Function *OutlinedFn = PrivAI->getFunction();
@@ -595,6 +598,7 @@
 
   Builder.restoreIP(AfterIP);
   Builder.CreateRetVoid();
+  OMPBuilder.finalize();
 
   EXPECT_FALSE(verifyModule(*M, ()));
 
Index: llvm/lib/Transforms/Utils/CodeExtractor.cpp
===
--- llvm/lib/Transforms/Utils/CodeExtractor.cpp
+++ llvm/lib/Transforms/Utils/CodeExtractor.cpp
@@ -1405,11 +1405,7 @@
   DISubprogram *OldSP = OldFunc.getSubprogram();
   LLVMContext  = OldFunc.getContext();
 
-  // See llvm.org/PR44560, OpenMP passes an invalid subprogram to CodeExtractor.
-  bool NeedWorkaroundForOpenMPIRBuilderBug =
-  OldSP && OldSP->getRetainedNodes()->isTemporary();
-
-  if (!OldSP || NeedWorkaroundForOpenMPIRBuilderBug) {
+  if (!OldSP) {
 // Erase any debug info the new function contains.
 stripDebugInfo(NewFunc);
 // Make sure the old function doesn't contain any non-local metadata refs.
Index: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
===
--- llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
+++ llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
@@ -93,6 +93,55 @@
 
 void OpenMPIRBuilder::initialize() { initializeTypes(M); }
 
+void OpenMPIRBuilder::finalize() {
+  for (OutlineInfo  : OutlineInfos) {
+assert(!OI.Blocks.empty() &&
+   "Outlined regions should have at least a single block!");
+BasicBlock *RegEntryBB = OI.Blocks.front();
+Function *OuterFn = RegEntryBB->getParent();
+CodeExtractorAnalysisCache CEAC(*OuterFn);
+CodeExtractor Extractor(OI.Blocks, /* DominatorTree */ nullptr,
+/* AggregateArgs */ false,
+/* BlockFrequencyInfo */ nullptr,
+/* BranchProbabilityInfo */ nullptr,
+/* AssumptionCache */ nullptr,
+/* AllowVarArgs */ true,
+/* AllowAlloca */ true,
+/* Suffix */ ".omp_par");
+
+LLVM_DEBUG(dbgs() << "Before outlining: " << *OuterFn << "\n");
+
+

[clang] 99c5bcb - Revert "Change clang option -ffp-model=precise to select ffp-contract=on"

[Melanie Blower via cfe-commits]

Author: Melanie Blower
Date: 2020-02-11T14:20:00-08:00
New Revision: 99c5bcbce89f07e68ccd89891a0300346705d013

URL: 
https://github.com/llvm/llvm-project/commit/99c5bcbce89f07e68ccd89891a0300346705d013
DIFF: 
https://github.com/llvm/llvm-project/commit/99c5bcbce89f07e68ccd89891a0300346705d013.diff

LOG: Revert "Change clang option -ffp-model=precise to select ffp-contract=on"

This reverts commit 3fcdf2fa945aca5849c5587c55de4186c7d81b8a.
Sorry I was too hasty with my commit, I will review Andy's comments
and resubmit.

Added: 


Modified: 
clang/docs/UsersManual.rst
clang/lib/Driver/ToolChains/Clang.cpp
clang/test/CodeGen/ppc-emmintrin.c
clang/test/CodeGen/ppc-xmmintrin.c
clang/test/Driver/fp-model.c

Removed: 




diff  --git a/clang/docs/UsersManual.rst b/clang/docs/UsersManual.rst
index 6c8c9f802082..856d5e34bbcc 100644
--- a/clang/docs/UsersManual.rst
+++ b/clang/docs/UsersManual.rst
@@ -1190,50 +1190,8 @@ installed.
 Controlling Floating Point Behavior
 ---
 
-Clang provides a number of ways to control floating point behavior, including
-with command line options and source pragmas. This section
-describes the various floating point semantic modes and the corresponding 
options.
-
-.. csv-table:: Floating Point Semantic Modes
-  :header: "Mode", "Values"
-  :widths: 15, 30, 30
-
-  "except_behavior", "{ignore, strict, may_trap}", "ffp-exception-behavior"
-  "fenv_access", "{off, on}", "(none)"
-  "rounding_mode", "{dynamic, tonearest, downward, upward, towardzero}", 
"frounding-math"
-  "contract", "{on, off, fast}", "ffp-contract"
-  "denormal_fp_math", "{IEEE, PreserveSign, PositiveZero}", "fdenormal-fp-math"
-  "denormal_fp32_math", "{IEEE, PreserveSign, PositiveZero}", 
"fdenormal-fp-math-fp32"
-  "support_math_errno", "{on, off}", "fmath-errno"
-  "no_honor_nans", "{on, off}", "fhonor-nans"
-  "no_honor_infinities", "{on, off}", "fhonor-infinities"
-  "no_signed_zeros", "{on, off}", "fsigned-zeros"
-  "allow_reciprocal", "{on, off}", "freciprocal-math"
-  "allow_approximate_fns", "{on, off}", "(none)"
-  "allow_reassociation", "{on, off}", "fassociative-math"
-
-
-This table describes the option settings that correspond to the three
-floating point semantic models: precise (the default), strict, and fast.
-
-
-.. csv-table:: Floating Point Models
-  :header: "Mode", "Precise", "Strict", "Fast"
-  :widths: 25, 15, 15, 15
-
-  "except_behavior", "ignore", "strict", "ignore"
-  "fenv_access", "off", "on", "off"
-  "rounding_mode", "tonearest", "dynamic", "tonearest"
-  "contract", "on", "off", "fast"
-  "denormal_fp_math", "IEEE", "IEEE", "PreserveSign"
-  "denormal_fp32_math", "IEEE","IEEE", "PreserveSign"
-  "support_math_errno", "on", "on", "off"
-  "no_honor_nans", "off", "off", "on"
-  "no_honor_infinities", "off", "off", "on"
-  "no_signed_zeros", "off", "off", "on"
-  "allow_reciprocal", "off", "off", "on"
-  "allow_approximate_fns", "off", "off", "on"
-  "allow_reassociation", "off", "off", "on"
+Clang provides a number of ways to control floating point behavior. The options
+are listed below.
 
 .. option:: -ffast-math
 
@@ -1427,7 +1385,7 @@ Note that floating-point operations performed as part of 
constant initialization
and ``fast``.
Details:
 
-   * ``precise`` Disables optimizations that are not value-safe on 
floating-point data, although FP contraction (FMA) is enabled 
(``-ffp-contract=on``).  This is the default behavior.
+   * ``precise`` Disables optimizations that are not value-safe on 
floating-point data, although FP contraction (FMA) is enabled 
(``-ffp-contract=fast``).  This is the default behavior.
* ``strict`` Enables ``-frounding-math`` and 
``-ffp-exception-behavior=strict``, and disables contractions (FMA).  All of 
the ``-ffast-math`` enablements are disabled.
* ``fast`` Behaves identically to specifying both ``-ffast-math`` and 
``ffp-contract=fast``
 

diff  --git a/clang/lib/Driver/ToolChains/Clang.cpp 
b/clang/lib/Driver/ToolChains/Clang.cpp
index c70659bf1485..7901f8a48f5f 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -2520,9 +2520,10 @@ static void RenderFloatingPointOptions(const ToolChain 
, const Driver ,
 
   llvm::DenormalMode DenormalFPMath = DefaultDenormalFPMath;
   llvm::DenormalMode DenormalFP32Math = DefaultDenormalFP32Math;
-  StringRef FPContract = "on";
+  StringRef FPContract = "";
   bool StrictFPModel = false;
 
+
   if (const Arg *A = Args.getLastArg(options::OPT_flimited_precision_EQ)) {
 CmdArgs.push_back("-mlimit-float-precision");
 CmdArgs.push_back(A->getValue());
@@ -2559,10 +2560,12 @@ static void RenderFloatingPointOptions(const ToolChain 
, const Driver ,
   // ffp-model= is a Driver option, it is entirely rewritten into more
   // granular options before being passed into cc1.
   // Use the gcc option in the 

[PATCH] D74436: Change clang option -ffp-model=precise to select ffp-contract=on

[Andy Kaylor via Phabricator via cfe-commits]

andrew.w.kaylor added inline comments.



Comment at: clang/docs/UsersManual.rst:1388
 
-   * ``precise`` Disables optimizations that are not value-safe on 
floating-point data, although FP contraction (FMA) is enabled 
(``-ffp-contract=fast``).  This is the default behavior.
* ``strict`` Enables ``-frounding-math`` and 
``-ffp-exception-behavior=strict``, and disables contractions (FMA).  All of 
the ``-ffast-math`` enablements are disabled.

lebedev.ri wrote:
> I'm confused. Where in this patch the `This patch establishes the default 
> option for -ffp-model to select "precise".` happens? LHS of diff says it is 
> already default
The comments said that it was the default, but the actual default was something 
that didn't quite match any of the fp-models -- precise but with fp contraction 
off.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:2548
   DenormalFPMath = DefaultDenormalFPMath;
   FPContract = "";
   StringRef Val = A->getValue();

I think this always gets changed to fast, on, or off below, but making it empty 
here looks wrong.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:2661
 // the FPContract value has already been set to a string literal
 // and the Val string isn't a pertinent value.
 ;

Does this mean that "-ffp-model=precise -ffp-contract=off" will leave FP 
contraction on? That doesn't seem right.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:2769
 DenormalFPMath != llvm::DenormalMode::getIEEE() &&
-FPContract.empty())
+(FPContract.equals("off") || FPContract.empty()))
 // OK: Current Arg doesn't conflict with -ffp-model=strict

We should never get here with FPContract empty.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74436



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

[PATCH] D74436: Change clang option -ffp-model=precise to select ffp-contract=on

[Melanie Blower via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
mibintc marked 2 inline comments as done.
Closed by commit rG3fcdf2fa945a: Change clang option -ffp-model=precise to 
select ffp-contract=on (authored by mibintc).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74436

Files:
  clang/docs/UsersManual.rst
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/CodeGen/ppc-emmintrin.c
  clang/test/CodeGen/ppc-xmmintrin.c
  clang/test/Driver/fp-model.c

Index: clang/test/Driver/fp-model.c
===
--- clang/test/Driver/fp-model.c
+++ clang/test/Driver/fp-model.c
@@ -27,9 +27,9 @@
 // RUN:   | FileCheck --check-prefix=WARN5 %s
 // WARN5: warning: overriding '-ffp-model=strict' option with '-ffp-contract=fast' [-Woverriding-t-option]
 
-// RUN: %clang -### -ffp-model=strict -ffp-contract=off -c %s 2>&1 \
+// RUN: %clang -### -ffp-model=strict -ffp-contract=fast -c %s 2>&1 \
 // RUN:   | FileCheck --check-prefix=WARN6 %s
-// WARN6: warning: overriding '-ffp-model=strict' option with '-ffp-contract=off' [-Woverriding-t-option]
+// WARN6: warning: overriding '-ffp-model=strict' option with '-ffp-contract=fast' [-Woverriding-t-option]
 
 // RUN: %clang -### -ffp-model=strict -ffp-contract=on -c %s 2>&1 \
 // RUN:   | FileCheck --check-prefix=WARN7 %s
@@ -100,13 +100,14 @@
 // RUN: %clang -### -nostdinc -ffp-model=precise -c %s 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-FPM-PRECISE %s
 // CHECK-FPM-PRECISE: "-cc1"
-// CHECK-FPM-PRECISE: "-ffp-contract=fast"
+// CHECK-FPM-PRECISE: "-ffp-contract=on"
 // CHECK-FPM-PRECISE: "-fno-rounding-math"
 
 // RUN: %clang -### -nostdinc -ffp-model=strict -c %s 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-FPM-STRICT %s
 // CHECK-FPM-STRICT: "-cc1"
 // CHECK-FPM-STRICT: "-ftrapping-math"
+// CHECK-FPM-STRICT: "-ffp-contract=off"
 // CHECK-FPM-STRICT: "-frounding-math"
 // CHECK-FPM-STRICT: "-ffp-exception-behavior=strict"
 
Index: clang/test/CodeGen/ppc-xmmintrin.c
===
--- clang/test/CodeGen/ppc-xmmintrin.c
+++ clang/test/CodeGen/ppc-xmmintrin.c
@@ -2,9 +2,9 @@
 // REQUIRES: powerpc-registered-target
 
 // RUN: %clang -S -emit-llvm -target powerpc64-unknown-linux-gnu -mcpu=pwr8 -ffreestanding -DNO_WARN_X86_INTRINSICS %s \
-// RUN:   -fno-discard-value-names -mllvm -disable-llvm-optzns -o - | llvm-cxxfilt -n | FileCheck %s --check-prefixes=CHECK,CHECK-BE
+// RUN:   -ffp-contract=off -fno-discard-value-names -mllvm -disable-llvm-optzns -o - | llvm-cxxfilt -n | FileCheck %s --check-prefixes=CHECK,CHECK-BE
 // RUN: %clang -S -emit-llvm -target powerpc64le-unknown-linux-gnu -mcpu=pwr8 -ffreestanding -DNO_WARN_X86_INTRINSICS %s \
-// RUN:   -fno-discard-value-names -mllvm -disable-llvm-optzns -o - | llvm-cxxfilt -n | FileCheck %s --check-prefixes=CHECK,CHECK-LE
+// RUN:   -ffp-contract=off -fno-discard-value-names -mllvm -disable-llvm-optzns -o - | llvm-cxxfilt -n | FileCheck %s --check-prefixes=CHECK,CHECK-LE
 
 #include 
 
Index: clang/test/CodeGen/ppc-emmintrin.c
===
--- clang/test/CodeGen/ppc-emmintrin.c
+++ clang/test/CodeGen/ppc-emmintrin.c
@@ -2,9 +2,9 @@
 // REQUIRES: powerpc-registered-target
 
 // RUN: %clang -S -emit-llvm -target powerpc64-unknown-linux-gnu -mcpu=pwr8 -ffreestanding -DNO_WARN_X86_INTRINSICS %s \
-// RUN:  -fno-discard-value-names -mllvm -disable-llvm-optzns -o - | llvm-cxxfilt -n | FileCheck %s --check-prefixes=CHECK,CHECK-BE
+// RUN:  -ffp-contract=off -fno-discard-value-names -mllvm -disable-llvm-optzns -o - | llvm-cxxfilt -n | FileCheck %s --check-prefixes=CHECK,CHECK-BE
 // RUN: %clang -S -emit-llvm -target powerpc64le-unknown-linux-gnu -mcpu=pwr8 -ffreestanding -DNO_WARN_X86_INTRINSICS %s \
-// RUN:   -fno-discard-value-names -mllvm -disable-llvm-optzns -o - | llvm-cxxfilt -n | FileCheck %s --check-prefixes=CHECK,CHECK-LE
+// RUN:   -ffp-contract=off -fno-discard-value-names -mllvm -disable-llvm-optzns -o - | llvm-cxxfilt -n | FileCheck %s --check-prefixes=CHECK,CHECK-LE
 
 // CHECK-BE-DAG: @_mm_movemask_pd.perm_mask = internal constant <4 x i32> , align 16
 // CHECK-BE-DAG: @_mm_shuffle_epi32.permute_selectors = internal constant [4 x i32] [i32 66051, i32 67438087, i32 134810123, i32 202182159], align 4
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -2520,10 +2520,9 @@
 
   llvm::DenormalMode DenormalFPMath = DefaultDenormalFPMath;
   llvm::DenormalMode DenormalFP32Math = DefaultDenormalFP32Math;
-  StringRef FPContract = "";
+  StringRef FPContract = "on";
   bool StrictFPModel = false;
 
-
   if (const Arg *A = Args.getLastArg(options::OPT_flimited_precision_EQ)) {
 

[clang] 3fcdf2f - Change clang option -ffp-model=precise to select ffp-contract=on

[Melanie Blower via cfe-commits]

Author: Melanie Blower
Date: 2020-02-11T14:07:10-08:00
New Revision: 3fcdf2fa945aca5849c5587c55de4186c7d81b8a

URL: 
https://github.com/llvm/llvm-project/commit/3fcdf2fa945aca5849c5587c55de4186c7d81b8a
DIFF: 
https://github.com/llvm/llvm-project/commit/3fcdf2fa945aca5849c5587c55de4186c7d81b8a.diff

LOG: Change clang option -ffp-model=precise to select ffp-contract=on

Reviewers: rjmccall

Differential Revision: https://reviews.llvm.org/D74436

Added: 


Modified: 
clang/docs/UsersManual.rst
clang/lib/Driver/ToolChains/Clang.cpp
clang/test/CodeGen/ppc-emmintrin.c
clang/test/CodeGen/ppc-xmmintrin.c
clang/test/Driver/fp-model.c

Removed: 




diff  --git a/clang/docs/UsersManual.rst b/clang/docs/UsersManual.rst
index 856d5e34bbcc..6c8c9f802082 100644
--- a/clang/docs/UsersManual.rst
+++ b/clang/docs/UsersManual.rst
@@ -1190,8 +1190,50 @@ installed.
 Controlling Floating Point Behavior
 ---
 
-Clang provides a number of ways to control floating point behavior. The options
-are listed below.
+Clang provides a number of ways to control floating point behavior, including
+with command line options and source pragmas. This section
+describes the various floating point semantic modes and the corresponding 
options.
+
+.. csv-table:: Floating Point Semantic Modes
+  :header: "Mode", "Values"
+  :widths: 15, 30, 30
+
+  "except_behavior", "{ignore, strict, may_trap}", "ffp-exception-behavior"
+  "fenv_access", "{off, on}", "(none)"
+  "rounding_mode", "{dynamic, tonearest, downward, upward, towardzero}", 
"frounding-math"
+  "contract", "{on, off, fast}", "ffp-contract"
+  "denormal_fp_math", "{IEEE, PreserveSign, PositiveZero}", "fdenormal-fp-math"
+  "denormal_fp32_math", "{IEEE, PreserveSign, PositiveZero}", 
"fdenormal-fp-math-fp32"
+  "support_math_errno", "{on, off}", "fmath-errno"
+  "no_honor_nans", "{on, off}", "fhonor-nans"
+  "no_honor_infinities", "{on, off}", "fhonor-infinities"
+  "no_signed_zeros", "{on, off}", "fsigned-zeros"
+  "allow_reciprocal", "{on, off}", "freciprocal-math"
+  "allow_approximate_fns", "{on, off}", "(none)"
+  "allow_reassociation", "{on, off}", "fassociative-math"
+
+
+This table describes the option settings that correspond to the three
+floating point semantic models: precise (the default), strict, and fast.
+
+
+.. csv-table:: Floating Point Models
+  :header: "Mode", "Precise", "Strict", "Fast"
+  :widths: 25, 15, 15, 15
+
+  "except_behavior", "ignore", "strict", "ignore"
+  "fenv_access", "off", "on", "off"
+  "rounding_mode", "tonearest", "dynamic", "tonearest"
+  "contract", "on", "off", "fast"
+  "denormal_fp_math", "IEEE", "IEEE", "PreserveSign"
+  "denormal_fp32_math", "IEEE","IEEE", "PreserveSign"
+  "support_math_errno", "on", "on", "off"
+  "no_honor_nans", "off", "off", "on"
+  "no_honor_infinities", "off", "off", "on"
+  "no_signed_zeros", "off", "off", "on"
+  "allow_reciprocal", "off", "off", "on"
+  "allow_approximate_fns", "off", "off", "on"
+  "allow_reassociation", "off", "off", "on"
 
 .. option:: -ffast-math
 
@@ -1385,7 +1427,7 @@ Note that floating-point operations performed as part of 
constant initialization
and ``fast``.
Details:
 
-   * ``precise`` Disables optimizations that are not value-safe on 
floating-point data, although FP contraction (FMA) is enabled 
(``-ffp-contract=fast``).  This is the default behavior.
+   * ``precise`` Disables optimizations that are not value-safe on 
floating-point data, although FP contraction (FMA) is enabled 
(``-ffp-contract=on``).  This is the default behavior.
* ``strict`` Enables ``-frounding-math`` and 
``-ffp-exception-behavior=strict``, and disables contractions (FMA).  All of 
the ``-ffast-math`` enablements are disabled.
* ``fast`` Behaves identically to specifying both ``-ffast-math`` and 
``ffp-contract=fast``
 

diff  --git a/clang/lib/Driver/ToolChains/Clang.cpp 
b/clang/lib/Driver/ToolChains/Clang.cpp
index 7901f8a48f5f..c70659bf1485 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -2520,10 +2520,9 @@ static void RenderFloatingPointOptions(const ToolChain 
, const Driver ,
 
   llvm::DenormalMode DenormalFPMath = DefaultDenormalFPMath;
   llvm::DenormalMode DenormalFP32Math = DefaultDenormalFP32Math;
-  StringRef FPContract = "";
+  StringRef FPContract = "on";
   bool StrictFPModel = false;
 
-
   if (const Arg *A = Args.getLastArg(options::OPT_flimited_precision_EQ)) {
 CmdArgs.push_back("-mlimit-float-precision");
 CmdArgs.push_back(A->getValue());
@@ -2560,12 +2559,10 @@ static void RenderFloatingPointOptions(const ToolChain 
, const Driver ,
   // ffp-model= is a Driver option, it is entirely rewritten into more
   // granular options before being passed into cc1.
   // Use the gcc option in the switch below.
-  if (!FPModel.empty() && !FPModel.equals(Val)) {
+  

[PATCH] D73842: [xray][clang] Always add xray-skip-entry/exit and xray-ignore-loops attrs

[Shoaib Meenai via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG14f870366a93: [xray][clang] Always add xray-skip-entry/exit 
and xray-ignore-loops attrs (authored by ianlevesque, committed by smeenai).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73842

Files:
  clang/include/clang/Driver/XRayArgs.h
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/Driver/XRayArgs.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGen/xray-attributes-skip-entry-exit.cpp
  clang/test/CodeGen/xray-ignore-loops.cpp
  clang/test/Driver/XRay/xray-ignore-loops-flags.cpp

Index: clang/test/Driver/XRay/xray-ignore-loops-flags.cpp
===
--- /dev/null
+++ clang/test/Driver/XRay/xray-ignore-loops-flags.cpp
@@ -0,0 +1,10 @@
+// This test ensures that when we invoke the clang compiler, that the -cc1
+// options include the -fxray-ignore-loops flag we provide in the
+// invocation.
+//
+// RUN: %clang -fxray-instrument -fxray-ignore-loops -target x86_64-linux- -### \
+// RUN: -x c++ -std=c++11 -emit-llvm -c -o - %s 2>&1 \
+// RUN: | FileCheck %s
+// CHECK:  -fxray-ignore-loops
+//
+// REQUIRES: x86_64 || x86_64h
Index: clang/test/CodeGen/xray-ignore-loops.cpp
===
--- clang/test/CodeGen/xray-ignore-loops.cpp
+++ clang/test/CodeGen/xray-ignore-loops.cpp
@@ -4,5 +4,5 @@
   return 1;
 }
 
-// CHECK: define i32 @_Z3foov() #[[ATTRS:[0-9]+]] {
+// CHECK: define{{.*}} i32 @_Z3foov() #[[ATTRS:[0-9]+]] {
 // CHECK-DAG: attributes #[[ATTRS]] = {{.*}} "xray-ignore-loops" {{.*}}
Index: clang/test/CodeGen/xray-attributes-skip-entry-exit.cpp
===
--- /dev/null
+++ clang/test/CodeGen/xray-attributes-skip-entry-exit.cpp
@@ -0,0 +1,22 @@
+// RUN: %clang_cc1 -fxray-instrument \
+// RUN: -fxray-instrumentation-bundle=function-entry -x c++ \
+// RUN: -std=c++11 -triple x86_64-unknown-unknown -emit-llvm -o - %s \
+// RUN: | FileCheck --check-prefixes CHECK,SKIPEXIT %s
+// RUN: %clang_cc1 -fxray-instrument \
+// RUN: -fxray-instrumentation-bundle=function-exit -x c++ \
+// RUN: -std=c++11 -triple x86_64-unknown-unknown -emit-llvm -o - %s \
+// RUN: | FileCheck --check-prefixes CHECK,SKIPENTRY %s
+// RUN: %clang_cc1 -fxray-instrument \
+// RUN: -fxray-instrumentation-bundle=function-entry,function-exit -x c++ \
+// RUN: -std=c++11 -triple x86_64-unknown-unknown -emit-llvm -o - %s \
+// RUN: | FileCheck --check-prefixes CHECK,NOSKIPENTRY,NOSKIPEXIT %s
+
+// CHECK: define void @_Z13justAFunctionv() #[[ATTR:[0-9]+]] {
+void justAFunction() {
+}
+
+// SKIPENTRY: attributes #[[ATTR]] = {{.*}} "xray-skip-entry" {{.*}}
+// SKIPEXIT: attributes #[[ATTR]] = {{.*}} "xray-skip-exit" {{.*}}
+
+// NOSKIPENTRY-NOT: attributes #[[ATTR]] = {{.*}} "xray-skip-entry" {{.*}}
+// NOSKIPEXIT-NOT: attributes #[[ATTR]] = {{.*}} "xray-skip-exit" {{.*}}
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -1099,8 +1099,7 @@
   Args.hasArg(OPT_fxray_always_emit_typedevents);
   Opts.XRayInstructionThreshold =
   getLastArgIntValue(Args, OPT_fxray_instruction_threshold_EQ, 200, Diags);
-  Opts.XRayIgnoreLoops =
-  Args.hasArg(OPT_fxray_ignore_loops, OPT_fno_xray_ignore_loops, false);
+  Opts.XRayIgnoreLoops = Args.hasArg(OPT_fxray_ignore_loops);
 
   auto XRayInstrBundles =
   Args.getAllArgValues(OPT_fxray_instrumentation_bundle);
Index: clang/lib/Driver/XRayArgs.cpp
===
--- clang/lib/Driver/XRayArgs.cpp
+++ clang/lib/Driver/XRayArgs.cpp
@@ -101,6 +101,10 @@
 options::OPT_fnoxray_link_deps, true))
 XRayRT = false;
 
+  if (Args.hasFlag(options::OPT_fxray_ignore_loops,
+   options::OPT_fno_xray_ignore_loops, false))
+XRayIgnoreLoops = true;
+
   auto Bundles =
   Args.getAllArgValues(options::OPT_fxray_instrumentation_bundle);
   if (Bundles.empty())
@@ -197,6 +201,9 @@
   if (XRayAlwaysEmitTypedEvents)
 CmdArgs.push_back("-fxray-always-emit-typedevents");
 
+  if (XRayIgnoreLoops)
+CmdArgs.push_back("-fxray-ignore-loops");
+
   CmdArgs.push_back(Args.MakeArgString(Twine(XRayInstructionThresholdOption) +
Twine(InstructionThreshold)));
 
Index: clang/lib/CodeGen/CodeGenFunction.cpp
===
--- clang/lib/CodeGen/CodeGenFunction.cpp
+++ clang/lib/CodeGen/CodeGenFunction.cpp
@@ -814,23 +814,25 @@
   if (ShouldXRayInstrumentFunction())
 Fn->addFnAttr("xray-log-args",
   

[PATCH] D74433: [llvm-objdump] Print file format in lowercase to match GNU output.

[Jordan Rupprecht via Phabricator via cfe-commits]

rupprecht updated this revision to Diff 243991.
rupprecht marked an inline comment as done.
rupprecht added a comment.

- Use StringRef::lower()


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74433

Files:
  clang/test/Modules/pch_container.m
  lld/test/COFF/savetemps.ll
  llvm/test/CodeGen/AArch64/arm64-simplest-elf.ll
  llvm/test/CodeGen/ARM/Windows/trivial-gnu-object.ll
  llvm/test/CodeGen/BPF/reloc-btf-2.ll
  llvm/test/CodeGen/BPF/reloc-btf.ll
  llvm/test/CodeGen/BPF/reloc.ll
  llvm/test/Object/AMDGPU/objdump.s
  llvm/test/Object/X86/objdump-disassembly-inline-relocations.test
  llvm/test/Object/X86/objdump-label.test
  llvm/test/Object/X86/objdump-trivial-object.test
  llvm/test/Object/dynamic-reloc.test
  llvm/test/Object/objdump-symbol-table.test
  llvm/test/tools/llvm-objdump/X86/disassemble-section-name.s
  llvm/test/tools/llvm-objdump/X86/elf-disassemble-symbol-labels-exec.test
  llvm/test/tools/llvm-objdump/X86/elf-dynamic-relocs.test
  llvm/test/tools/llvm-objdump/X86/output-ordering.test
  llvm/test/tools/llvm-objdump/X86/warn-missing-disasm-func.test
  llvm/test/tools/llvm-objdump/all-headers.test
  llvm/test/tools/llvm-objdump/archive-headers.test
  llvm/test/tools/llvm-objdump/file-headers-coff.test
  llvm/test/tools/llvm-objdump/file-headers-elf.test
  llvm/test/tools/llvm-objdump/non-archive-object.test
  llvm/test/tools/llvm-objdump/relocations-in-nonreloc.test
  llvm/tools/llvm-objdump/llvm-objdump.cpp

Index: llvm/tools/llvm-objdump/llvm-objdump.cpp
===
--- llvm/tools/llvm-objdump/llvm-objdump.cpp
+++ llvm/tools/llvm-objdump/llvm-objdump.cpp
@@ -2172,7 +2172,7 @@
   outs() << A->getFileName() << "(" << O->getFileName() << ")";
 else
   outs() << O->getFileName();
-outs() << ":\tfile format " << O->getFileFormatName() << "\n\n";
+outs() << ":\tfile format " << O->getFileFormatName().lower() << "\n\n";
   }
 
   if (StartAddress.getNumOccurrences() || StopAddress.getNumOccurrences())
Index: llvm/test/tools/llvm-objdump/relocations-in-nonreloc.test
===
--- llvm/test/tools/llvm-objdump/relocations-in-nonreloc.test
+++ llvm/test/tools/llvm-objdump/relocations-in-nonreloc.test
@@ -7,7 +7,7 @@
 # RUN: yaml2obj --docnum=3 %s -o %t3
 # RUN: llvm-objdump -r %t3 | FileCheck %s -DFILE=%t3 --check-prefixes=FMT,REL --implicit-check-not={{.}}
 
-# FMT: [[FILE]]: file format ELF64-x86-64
+# FMT: [[FILE]]: file format elf64-x86-64
 
 # REL:  RELOCATION RECORDS FOR []:
 # REL-NEXT: 0123 R_X86_64_NONE *ABS*+0x141
Index: llvm/test/tools/llvm-objdump/non-archive-object.test
===
--- llvm/test/tools/llvm-objdump/non-archive-object.test
+++ llvm/test/tools/llvm-objdump/non-archive-object.test
@@ -2,7 +2,7 @@
 # RUN: llvm-objdump -a %t | FileCheck %s
 
 # If this test has not crashed, then this test passed.
-# CHECK: file format ELF64-x86-64
+# CHECK: file format elf64-x86-64
 
 !ELF
 FileHeader:
Index: llvm/test/tools/llvm-objdump/file-headers-elf.test
===
--- llvm/test/tools/llvm-objdump/file-headers-elf.test
+++ llvm/test/tools/llvm-objdump/file-headers-elf.test
@@ -10,7 +10,7 @@
   Machine: EM_X86_64
   Entry:   0x123456789abcde
 
-# ELF64: [[FILE]]: file format ELF64-x86-64
+# ELF64: [[FILE]]: file format elf64-x86-64
 # ELF64: architecture: x86_64
 # ELF64: start address: 0x00123456789abcde
 
@@ -18,7 +18,7 @@
 # RUN: llvm-objdump -f %t-i386 | FileCheck -DFILE=%t-i386 %s -check-prefix ELF32
 # RUN: llvm-objdump -file-headers %t-i386 | FileCheck %s -DFILE=%t-i386 -check-prefix ELF32
 
-# ELF32: [[FILE]]: file format ELF32-i386
+# ELF32: [[FILE]]: file format elf32-i386
 # ELF32: architecture: i386
 # ELF32: start address: 0x12345678
 
Index: llvm/test/tools/llvm-objdump/file-headers-coff.test
===
--- llvm/test/tools/llvm-objdump/file-headers-coff.test
+++ llvm/test/tools/llvm-objdump/file-headers-coff.test
@@ -9,6 +9,6 @@
 sections:
 symbols:
 
-# CHECK: [[FILE]]: file format COFF-i386
+# CHECK: [[FILE]]: file format coff-i386
 # CHECK: architecture: i386
 # CHECK: start address: 0x
Index: llvm/test/tools/llvm-objdump/archive-headers.test
===
--- llvm/test/tools/llvm-objdump/archive-headers.test
+++ llvm/test/tools/llvm-objdump/archive-headers.test
@@ -1,21 +1,21 @@
 # RUN: llvm-objdump -a %p/Inputs/liblong_filenames.a | FileCheck %s
 # RUN: llvm-objdump -archive-headers %p/Inputs/liblong_filenames.a | FileCheck %s
 
-# CHECK: {{.*}}liblong_filenames.a(1.o): file format ELF64-x86-64
+# CHECK: {{.*}}liblong_filenames.a(1.o): file format elf64-x86-64
 # CHECK: rw-r--r-- 204299/200  1416 

[PATCH] D74433: [llvm-objdump] Print file format in lowercase to match GNU output.

[Jordan Rupprecht via Phabricator via cfe-commits]

rupprecht added a comment.

In D74433#1870647 , @MaskRay wrote:

> Wait. I wonder whether we can change llvm-readobj to use lower case names as 
> well. The following should be updated:
>
>   StringRef ELFObjectFile::getFileFormatName() const {
> bool IsLittleEndian = ELFT::TargetEndianness == support::little;
> switch (EF.getHeader()->e_ident[ELF::EI_CLASS]) {
> case ELF::ELFCLASS32:
>   switch (EF.getHeader()->e_machine) {
>   case ELF::EM_386:
> return "ELF32-i386";
>   case ELF::EM_IAMCU:
> return "ELF32-iamcu";
>   case ELF::EM_X86_64:
> return "ELF32-x86-64";
>   case ELF::EM_ARM:
>
>
> These are almost bfdnames, except that they use upper cases. I don't find a 
> compelling argument using upper case `ELF`, so I vote for changing these 
> `ELF` to `elf`. @jhenderson  @grimar @sbc100 thoughts?
>
>   // lib/Object/WasmObjectFile.cpp
>   StringRef WasmObjectFile::getFileFormatName() const { return "WASM"; }
>


Yep... I was originally thinking we could update those to be lowercase, but 
that's kinda a broader change, and I imagine most llvm developers are used to 
seeing "ELF64-..." and I'm not sure it's worth changing all the tools just so 
that objdump can be more GNU compatible.

I personally wouldn't mind it. If others are in favor of it too, I'm all for it 
and can make the change.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74433



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

[PATCH] D73904: [clang] stop baremetal driver to append .a to lib

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

A probably better approach is to use something like `TC.getCompilerRT(Args, 
"profile")`. The function returns a full path which can avoid `-L` problems.

  std::string ToolChain::getCompilerRT(const ArgList , StringRef Component,
   FileType Type) const {
const llvm::Triple  = getTriple();
bool IsITANMSVCWindows =
TT.isWindowsMSVCEnvironment() || TT.isWindowsItaniumEnvironment();
  
const char *Prefix =
IsITANMSVCWindows || Type == ToolChain::FT_Object ? "" : "lib";
const char *Suffix;
switch (Type) {
case ToolChain::FT_Object:
  Suffix = IsITANMSVCWindows ? ".obj" : ".o";
  break;
case ToolChain::FT_Static:
  Suffix = IsITANMSVCWindows ? ".lib" : ".a";
  break;
case ToolChain::FT_Shared:
  Suffix = Triple.isOSWindows()
   ? (Triple.isWindowsGNUEnvironment() ? ".dll.a" : ".lib")
   : ".so";
  break;
}
  
for (const auto  : getLibraryPaths()) {
  SmallString<128> P(LibPath);
  llvm::sys::path::append(P, Prefix + Twine("clang_rt.") + Component + 
Suffix);
  if (getVFS().exists(P))
return std::string(P.str());
}
  
StringRef Arch = getArchNameForCompilerRTLib(*this, Args);
const char *Env = TT.isAndroid() ? "-android" : "";
SmallString<128> Path(getCompilerRTPath());
llvm::sys::path::append(Path, Prefix + Twine("clang_rt.") + Component + "-" 
+
  Arch + Env + Suffix);
return std::string(Path.str());
  }


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

https://reviews.llvm.org/D73904



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

[clang] 14f8703 - [xray][clang] Always add xray-skip-entry/exit and xray-ignore-loops attrs

[Shoaib Meenai via cfe-commits]

Author: Ian Levesque
Date: 2020-02-11T14:00:41-08:00
New Revision: 14f870366a93ba0c6311883d900e24339681ba76

URL: 
https://github.com/llvm/llvm-project/commit/14f870366a93ba0c6311883d900e24339681ba76
DIFF: 
https://github.com/llvm/llvm-project/commit/14f870366a93ba0c6311883d900e24339681ba76.diff

LOG: [xray][clang] Always add xray-skip-entry/exit and xray-ignore-loops attrs

The function attributes xray-skip-entry, xray-skip-exit, and
xray-ignore-loops were only being applied if a function had an
xray-instrument attribute, but they should apply if xray is enabled
globally too.

Differential Revision: https://reviews.llvm.org/D73842

Added: 
clang/test/CodeGen/xray-attributes-skip-entry-exit.cpp
clang/test/Driver/XRay/xray-ignore-loops-flags.cpp

Modified: 
clang/include/clang/Driver/XRayArgs.h
clang/lib/CodeGen/CodeGenFunction.cpp
clang/lib/Driver/XRayArgs.cpp
clang/lib/Frontend/CompilerInvocation.cpp
clang/test/CodeGen/xray-ignore-loops.cpp

Removed: 




diff  --git a/clang/include/clang/Driver/XRayArgs.h 
b/clang/include/clang/Driver/XRayArgs.h
index fa2583f4b966..96098bf629cd 100644
--- a/clang/include/clang/Driver/XRayArgs.h
+++ b/clang/include/clang/Driver/XRayArgs.h
@@ -30,6 +30,7 @@ class XRayArgs {
   bool XRayAlwaysEmitCustomEvents = false;
   bool XRayAlwaysEmitTypedEvents = false;
   bool XRayRT = true;
+  bool XRayIgnoreLoops = false;
 
 public:
   /// Parses the XRay arguments from an argument list.

diff  --git a/clang/lib/CodeGen/CodeGenFunction.cpp 
b/clang/lib/CodeGen/CodeGenFunction.cpp
index af0adc213580..b7506b53030d 100644
--- a/clang/lib/CodeGen/CodeGenFunction.cpp
+++ b/clang/lib/CodeGen/CodeGenFunction.cpp
@@ -814,23 +814,25 @@ void CodeGenFunction::StartFunction(GlobalDecl GD, 
QualType RetTy,
   if (ShouldXRayInstrumentFunction())
 Fn->addFnAttr("xray-log-args",
   llvm::utostr(LogArgs->getArgumentCount()));
-if (!CGM.getCodeGenOpts().XRayInstrumentationBundle.has(
-XRayInstrKind::FunctionExit)) {
-  Fn->addFnAttr("xray-skip-exit");
-}
-if (!CGM.getCodeGenOpts().XRayInstrumentationBundle.has(
-XRayInstrKind::FunctionEntry)) {
-  Fn->addFnAttr("xray-skip-entry");
-}
   }
 } else {
   if (ShouldXRayInstrumentFunction() && !CGM.imbueXRayAttrs(Fn, Loc))
 Fn->addFnAttr(
 "xray-instruction-threshold",
 llvm::itostr(CGM.getCodeGenOpts().XRayInstructionThreshold));
-  if (CGM.getCodeGenOpts().XRayIgnoreLoops) {
+}
+
+if (ShouldXRayInstrumentFunction()) {
+  if (CGM.getCodeGenOpts().XRayIgnoreLoops)
 Fn->addFnAttr("xray-ignore-loops");
-  }
+
+  if (!CGM.getCodeGenOpts().XRayInstrumentationBundle.has(
+  XRayInstrKind::FunctionExit))
+Fn->addFnAttr("xray-skip-exit");
+
+  if (!CGM.getCodeGenOpts().XRayInstrumentationBundle.has(
+  XRayInstrKind::FunctionEntry))
+Fn->addFnAttr("xray-skip-entry");
 }
 
 unsigned Count, Offset;

diff  --git a/clang/lib/Driver/XRayArgs.cpp b/clang/lib/Driver/XRayArgs.cpp
index 66c3959648ac..54c15685d389 100644
--- a/clang/lib/Driver/XRayArgs.cpp
+++ b/clang/lib/Driver/XRayArgs.cpp
@@ -101,6 +101,10 @@ XRayArgs::XRayArgs(const ToolChain , const ArgList 
) {
 options::OPT_fnoxray_link_deps, true))
 XRayRT = false;
 
+  if (Args.hasFlag(options::OPT_fxray_ignore_loops,
+   options::OPT_fno_xray_ignore_loops, false))
+XRayIgnoreLoops = true;
+
   auto Bundles =
   Args.getAllArgValues(options::OPT_fxray_instrumentation_bundle);
   if (Bundles.empty())
@@ -197,6 +201,9 @@ void XRayArgs::addArgs(const ToolChain , const ArgList 
,
   if (XRayAlwaysEmitTypedEvents)
 CmdArgs.push_back("-fxray-always-emit-typedevents");
 
+  if (XRayIgnoreLoops)
+CmdArgs.push_back("-fxray-ignore-loops");
+
   CmdArgs.push_back(Args.MakeArgString(Twine(XRayInstructionThresholdOption) +
Twine(InstructionThreshold)));
 

diff  --git a/clang/lib/Frontend/CompilerInvocation.cpp 
b/clang/lib/Frontend/CompilerInvocation.cpp
index e57c7ef55bb5..ae39066edd4c 100644
--- a/clang/lib/Frontend/CompilerInvocation.cpp
+++ b/clang/lib/Frontend/CompilerInvocation.cpp
@@ -1099,8 +1099,7 @@ static bool ParseCodeGenArgs(CodeGenOptions , 
ArgList , InputKind IK,
   Args.hasArg(OPT_fxray_always_emit_typedevents);
   Opts.XRayInstructionThreshold =
   getLastArgIntValue(Args, OPT_fxray_instruction_threshold_EQ, 200, Diags);
-  Opts.XRayIgnoreLoops =
-  Args.hasArg(OPT_fxray_ignore_loops, OPT_fno_xray_ignore_loops, false);
+  Opts.XRayIgnoreLoops = Args.hasArg(OPT_fxray_ignore_loops);
 
   auto XRayInstrBundles =
   Args.getAllArgValues(OPT_fxray_instrumentation_bundle);

diff  --git a/clang/test/CodeGen/xray-attributes-skip-entry-exit.cpp 

[PATCH] D74436: Change clang option -ffp-model=precise to select ffp-contract=on

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

I agree that the default mode should be standards-compliant, which I believe 
permits `fp-contract=on` (contraction within expressions) but not 
`fp-contract=fast` (arbitrary contraction).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74436



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

[PATCH] D74433: [llvm-objdump] Print file format in lowercase to match GNU output.

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

I think the patch is good to go on its own, after the `outs() << ":\tfile 
format " << O->getFileFormatName().lower() << "\n\n";` change.

If the consensus is to also change llvm-readobj output, we can teach 
`getFileFormatName()` to use lower case names and remove `.lower()` from 
llvm-objdump.cpp


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74433



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

[PATCH] D73842: [xray][clang] Always add xray-skip-entry/exit and xray-ignore-loops attrs

[Shoaib Meenai via Phabricator via cfe-commits]

smeenai accepted this revision.
smeenai added a comment.

LGTM. I'll commit this for you.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73842



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

[PATCH] D73842: [xray][clang] Always add xray-skip-entry/exit and xray-ignore-loops attrs

[Ian Levesque via Phabricator via cfe-commits]

ianlevesque added a comment.

In D73842#1869890 , @smeenai wrote:

> Is it worth adding a test that a function with an explicit xray-instrument 
> attribute also has these other attributes applied?


The existing tests from when the feature was added covered that case well. It 
was the driver passing the flag and the case without the xray-instrument 
attribute that weren't working.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73842



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

[PATCH] D74015: [AIX][Frontend] C++ ABI customizations for AIX boilerplate

[Sean Fertile via Phabricator via cfe-commits]

sfertile added inline comments.



Comment at: clang/include/clang/Basic/TargetCXXABI.h:116
+///   - static initialization is adjusted to use sinit and sterm functions;
+XL_Clang,
+

Xiangling_L wrote:
> daltenty wrote:
> > Why the underscore in the name? This is a bit inconsistent with both the 
> > LLVM naming convention here and the name as it appears in other sources.
> There are various AIX ABI. So to distinguish the one we are implementing, we 
> choose `XL` and `Clang` as two parts of the abi name. 
> `XL` - not g++;
> `Clang` - it's a  ABI implemented in Clang;
> 
> And also `XLClang` is misleading because it represents our AIX XL C/C++ 
> compiler itself externally.
So do we need the 'Clang' part in the name? For example the ABI below is not 
`Microsoft_Clang`. Or is the `_Clang` differentiating between multiple XL ABIs?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74015



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

[PATCH] D74372: [OpenMP][IRBuilder] Perform finalization (incl. outlining) late

[Fady Ghanim via Phabricator via cfe-commits]

fghanim added inline comments.



Comment at: clang/lib/CodeGen/CodeGenFunction.cpp:113
+  if (llvm::OpenMPIRBuilder *OMPBuilder = CGM.getOpenMPIRBuilder())
+OMPBuilder->finalize();
 }

jdoerfert wrote:
> fghanim wrote:
> > jdoerfert wrote:
> > > fghanim wrote:
> > > > Does this mean that `finalize()` is being called twice everytime, here 
> > > > and `~OpenMPIRBuilder()`?
> > > > if yes, is there a reason why you want that?
> > > It's called twice now, not that it matters. The basic rule is, call it at 
> > > least once before you finalize the module. I'll remove the destructor for 
> > > now to make it explicit only.
> > Right now it doesn't matter, I agree. However, if later `finalize()` is 
> > modified for any reason, I worry this may introduce problems 
> > unintentionally.
> > 
> > In any case, if you decide to keep only one of them, I prefer to keep the 
> > one in `~OpenMPIRBuilder()`. This way the `OMPBuilder` is self contained, 
> > so regardless of the frontend using it, we can be certain it's always 
> > called at the end. You can indicate explicitly in the description comment 
> > for `finalize()` that it is being called in the destructor, if you want.
> That doesn't work, otherwise I would not call finalize here. At least it 
> doesn't work without extra changes. As it is right now, the OMPBuilder in 
> CodeGenModule is deleted *after* the IR is generated and emitted, so any 
> changes at destructor time are too late.
Got it. Then a minor suggestion is to just put a note in `finalize()` 
description about whatever you decide to do (i.e. keep both, remove the one in 
destructor, or something else entirely). This way, whoever is using/modifying 
the code is aware of it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74372



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

[PATCH] D73842: [xray][clang] Always add xray-skip-entry/exit and xray-ignore-loops attrs

[Ian Levesque via Phabricator via cfe-commits]

ianlevesque updated this revision to Diff 243981.
ianlevesque added a comment.

Address code review feedback on the tests.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73842

Files:
  clang/include/clang/Driver/XRayArgs.h
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/Driver/XRayArgs.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGen/xray-attributes-skip-entry-exit.cpp
  clang/test/CodeGen/xray-ignore-loops.cpp
  clang/test/Driver/XRay/xray-ignore-loops-flags.cpp

Index: clang/test/Driver/XRay/xray-ignore-loops-flags.cpp
===
--- /dev/null
+++ clang/test/Driver/XRay/xray-ignore-loops-flags.cpp
@@ -0,0 +1,10 @@
+// This test ensures that when we invoke the clang compiler, that the -cc1
+// options include the -fxray-ignore-loops flag we provide in the
+// invocation.
+//
+// RUN: %clang -fxray-instrument -fxray-ignore-loops -target x86_64-linux- -### \
+// RUN: -x c++ -std=c++11 -emit-llvm -c -o - %s 2>&1 \
+// RUN: | FileCheck %s
+// CHECK:  -fxray-ignore-loops
+//
+// REQUIRES: x86_64 || x86_64h
Index: clang/test/CodeGen/xray-ignore-loops.cpp
===
--- clang/test/CodeGen/xray-ignore-loops.cpp
+++ clang/test/CodeGen/xray-ignore-loops.cpp
@@ -4,5 +4,5 @@
   return 1;
 }
 
-// CHECK: define i32 @_Z3foov() #[[ATTRS:[0-9]+]] {
+// CHECK: define{{.*}} i32 @_Z3foov() #[[ATTRS:[0-9]+]] {
 // CHECK-DAG: attributes #[[ATTRS]] = {{.*}} "xray-ignore-loops" {{.*}}
Index: clang/test/CodeGen/xray-attributes-skip-entry-exit.cpp
===
--- /dev/null
+++ clang/test/CodeGen/xray-attributes-skip-entry-exit.cpp
@@ -0,0 +1,22 @@
+// RUN: %clang_cc1 -fxray-instrument \
+// RUN: -fxray-instrumentation-bundle=function-entry -x c++ \
+// RUN: -std=c++11 -triple x86_64-unknown-unknown -emit-llvm -o - %s \
+// RUN: | FileCheck --check-prefixes CHECK,SKIPEXIT %s
+// RUN: %clang_cc1 -fxray-instrument \
+// RUN: -fxray-instrumentation-bundle=function-exit -x c++ \
+// RUN: -std=c++11 -triple x86_64-unknown-unknown -emit-llvm -o - %s \
+// RUN: | FileCheck --check-prefixes CHECK,SKIPENTRY %s
+// RUN: %clang_cc1 -fxray-instrument \
+// RUN: -fxray-instrumentation-bundle=function-entry,function-exit -x c++ \
+// RUN: -std=c++11 -triple x86_64-unknown-unknown -emit-llvm -o - %s \
+// RUN: | FileCheck --check-prefixes CHECK,NOSKIPENTRY,NOSKIPEXIT %s
+
+// CHECK: define void @_Z13justAFunctionv() #[[ATTR:[0-9]+]] {
+void justAFunction() {
+}
+
+// SKIPENTRY: attributes #[[ATTR]] = {{.*}} "xray-skip-entry" {{.*}}
+// SKIPEXIT: attributes #[[ATTR]] = {{.*}} "xray-skip-exit" {{.*}}
+
+// NOSKIPENTRY-NOT: attributes #[[ATTR]] = {{.*}} "xray-skip-entry" {{.*}}
+// NOSKIPEXIT-NOT: attributes #[[ATTR]] = {{.*}} "xray-skip-exit" {{.*}}
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -1099,8 +1099,7 @@
   Args.hasArg(OPT_fxray_always_emit_typedevents);
   Opts.XRayInstructionThreshold =
   getLastArgIntValue(Args, OPT_fxray_instruction_threshold_EQ, 200, Diags);
-  Opts.XRayIgnoreLoops =
-  Args.hasArg(OPT_fxray_ignore_loops, OPT_fno_xray_ignore_loops, false);
+  Opts.XRayIgnoreLoops = Args.hasArg(OPT_fxray_ignore_loops);
 
   auto XRayInstrBundles =
   Args.getAllArgValues(OPT_fxray_instrumentation_bundle);
Index: clang/lib/Driver/XRayArgs.cpp
===
--- clang/lib/Driver/XRayArgs.cpp
+++ clang/lib/Driver/XRayArgs.cpp
@@ -101,6 +101,10 @@
 options::OPT_fnoxray_link_deps, true))
 XRayRT = false;
 
+  if (Args.hasFlag(options::OPT_fxray_ignore_loops,
+   options::OPT_fno_xray_ignore_loops, false))
+XRayIgnoreLoops = true;
+
   auto Bundles =
   Args.getAllArgValues(options::OPT_fxray_instrumentation_bundle);
   if (Bundles.empty())
@@ -197,6 +201,9 @@
   if (XRayAlwaysEmitTypedEvents)
 CmdArgs.push_back("-fxray-always-emit-typedevents");
 
+  if (XRayIgnoreLoops)
+CmdArgs.push_back("-fxray-ignore-loops");
+
   CmdArgs.push_back(Args.MakeArgString(Twine(XRayInstructionThresholdOption) +
Twine(InstructionThreshold)));
 
Index: clang/lib/CodeGen/CodeGenFunction.cpp
===
--- clang/lib/CodeGen/CodeGenFunction.cpp
+++ clang/lib/CodeGen/CodeGenFunction.cpp
@@ -814,23 +814,25 @@
   if (ShouldXRayInstrumentFunction())
 Fn->addFnAttr("xray-log-args",
   llvm::utostr(LogArgs->getArgumentCount()));
-if (!CGM.getCodeGenOpts().XRayInstrumentationBundle.has(
-

[PATCH] D74433: [llvm-objdump] Print file format in lowercase to match GNU output.

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a subscriber: grimar.
MaskRay added a comment.

Wait. I wonder whether we can change llvm-readobj to use lower case names as 
well. The following should be updated:

  StringRef ELFObjectFile::getFileFormatName() const {
bool IsLittleEndian = ELFT::TargetEndianness == support::little;
switch (EF.getHeader()->e_ident[ELF::EI_CLASS]) {
case ELF::ELFCLASS32:
  switch (EF.getHeader()->e_machine) {
  case ELF::EM_386:
return "ELF32-i386";
  case ELF::EM_IAMCU:
return "ELF32-iamcu";
  case ELF::EM_X86_64:
return "ELF32-x86-64";
  case ELF::EM_ARM:

I vote for changing these `ELF` to `elf`. @jhenderson  @grimar thoughts?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74433



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

[PATCH] D62686: [RISCV] Add support for save/restore of callee-saved registers via libcalls

[Lewis Revill via Phabricator via cfe-commits]

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rG07f7c00208b3: [RISCV] Add support for save/restore of 
callee-saved registers via libcalls (authored by lewis-revill).

Changed prior to commit:
  https://reviews.llvm.org/D62686?vs=242723=243978#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62686

Files:
  clang/lib/Driver/ToolChains/Arch/RISCV.cpp
  clang/test/Driver/riscv-features.c
  llvm/lib/Target/RISCV/RISCV.td
  llvm/lib/Target/RISCV/RISCVFrameLowering.cpp
  llvm/lib/Target/RISCV/RISCVFrameLowering.h
  llvm/lib/Target/RISCV/RISCVMachineFunctionInfo.h
  llvm/lib/Target/RISCV/RISCVRegisterInfo.cpp
  llvm/lib/Target/RISCV/RISCVRegisterInfo.h
  llvm/lib/Target/RISCV/RISCVSubtarget.h
  llvm/test/CodeGen/RISCV/saverestore.ll
  llvm/test/CodeGen/RISCV/shrinkwrap.ll

Index: llvm/test/CodeGen/RISCV/shrinkwrap.ll
===
--- llvm/test/CodeGen/RISCV/shrinkwrap.ll
+++ llvm/test/CodeGen/RISCV/shrinkwrap.ll
@@ -1,6 +1,8 @@
 ; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
 ; RUN: llc -mtriple riscv32 < %s | FileCheck %s -check-prefix=RV32I-NOSW
 ; RUN: llc -mtriple riscv32 -enable-shrink-wrap < %s | FileCheck %s -check-prefix=RV32I-SW
+; RUN: llc -mtriple riscv32 -enable-shrink-wrap -mattr=+save-restore < %s \
+; RUN: | FileCheck %s -check-prefix=RV32I-SW-SR
 
 
 declare void @abort()
@@ -29,6 +31,16 @@
 ; RV32I-SW-NEXT:addi sp, sp, -16
 ; RV32I-SW-NEXT:sw ra, 12(sp)
 ; RV32I-SW-NEXT:call abort
+;
+; RV32I-SW-SR-LABEL: eliminate_restore:
+; RV32I-SW-SR:   # %bb.0:
+; RV32I-SW-SR-NEXT:addi a1, zero, 32
+; RV32I-SW-SR-NEXT:bgeu a1, a0, .LBB0_2
+; RV32I-SW-SR-NEXT:  # %bb.1: # %if.end
+; RV32I-SW-SR-NEXT:ret
+; RV32I-SW-SR-NEXT:  .LBB0_2: # %if.then
+; RV32I-SW-SR-NEXT:call t0, __riscv_save_0
+; RV32I-SW-SR-NEXT:call abort
   %cmp = icmp ule i32 %n, 32
   br i1 %cmp, label %if.then, label %if.end
 
@@ -84,6 +96,23 @@
 ; RV32I-SW-NEXT:addi sp, sp, 16
 ; RV32I-SW-NEXT:  .LBB1_2: # %if.end
 ; RV32I-SW-NEXT:ret
+;
+; RV32I-SW-SR-LABEL: conditional_alloca:
+; RV32I-SW-SR:   # %bb.0:
+; RV32I-SW-SR-NEXT:addi a1, zero, 32
+; RV32I-SW-SR-NEXT:bltu a1, a0, .LBB1_2
+; RV32I-SW-SR-NEXT:  # %bb.1: # %if.then
+; RV32I-SW-SR-NEXT:call t0, __riscv_save_1
+; RV32I-SW-SR-NEXT:addi s0, sp, 16
+; RV32I-SW-SR-NEXT:addi a0, a0, 15
+; RV32I-SW-SR-NEXT:andi a0, a0, -16
+; RV32I-SW-SR-NEXT:sub a0, sp, a0
+; RV32I-SW-SR-NEXT:mv sp, a0
+; RV32I-SW-SR-NEXT:call notdead
+; RV32I-SW-SR-NEXT:addi sp, s0, -16
+; RV32I-SW-SR-NEXT:tail __riscv_restore_1
+; RV32I-SW-SR-NEXT:  .LBB1_2: # %if.end
+; RV32I-SW-SR-NEXT:ret
   %cmp = icmp ule i32 %n, 32
   br i1 %cmp, label %if.then, label %if.end
 
Index: llvm/test/CodeGen/RISCV/saverestore.ll
===
--- /dev/null
+++ llvm/test/CodeGen/RISCV/saverestore.ll
@@ -0,0 +1,299 @@
+; RUN: llc -mtriple=riscv32 < %s | FileCheck %s -check-prefix=RV32I
+; RUN: llc -mtriple=riscv64 < %s | FileCheck %s -check-prefix=RV64I
+; RUN: llc -mtriple=riscv32 -mattr=+save-restore < %s | FileCheck %s -check-prefix=RV32I-SR
+; RUN: llc -mtriple=riscv64 -mattr=+save-restore < %s | FileCheck %s -check-prefix=RV64I-SR
+; RUN: llc -mtriple=riscv32 -mattr=+f,+save-restore -target-abi=ilp32f < %s | FileCheck %s -check-prefix=RV32I-FP-SR
+; RUN: llc -mtriple=riscv64 -mattr=+f,+d,+save-restore -target-abi=lp64d < %s | FileCheck %s -check-prefix=RV64I-FP-SR
+
+; Check that the correct save/restore libcalls are generated.
+
+@var0 = global [18 x i32] zeroinitializer
+@var1 = global [24 x i32] zeroinitializer
+@var2 = global [30 x i32] zeroinitializer
+
+define void @callee_saved0() nounwind {
+; RV32I-LABEL: callee_saved0:
+; RV32I-NOT: call t0, __riscv_save
+; RV32I-NOT: tail __riscv_restore
+;
+; RV64I-LABEL: callee_saved0:
+; RV64I-NOT: call t0, __riscv_save
+; RV64I-NOT: tail __riscv_restore
+;
+; RV32I-SR-LABEL: callee_saved0:
+; RV32I-SR: call t0, __riscv_save_5
+; RV32I-SR: tail __riscv_restore_5
+;
+; RV64I-SR-LABEL: callee_saved0:
+; RV64I-SR: call t0, __riscv_save_5
+; RV64I-SR: tail __riscv_restore_5
+;
+; RV32I-FP-SR-LABEL: callee_saved0:
+; RV32I-FP-SR: call t0, __riscv_save_5
+; RV32I-FP-SR: tail __riscv_restore_5
+;
+; RV64I-FP-SR-LABEL: callee_saved0:
+; RV64I-FP-SR: call t0, __riscv_save_5
+; RV64I-FP-SR: tail __riscv_restore_5
+  %val = load [18 x i32], [18 x i32]* @var0
+  store volatile [18 x i32] %val, [18 x i32]* @var0
+  ret void
+}
+
+define void @callee_saved1() nounwind {
+; RV32I-LABEL: callee_saved1:
+; RV32I-NOT: call t0, __riscv_save
+; RV32I-NOT: tail 

[PATCH] D74015: [AIX][Frontend] C++ ABI customizations for AIX boilerplate

[Xiangling Liao via Phabricator via cfe-commits]

Xiangling_L marked 4 inline comments as done.
Xiangling_L added inline comments.



Comment at: clang/include/clang/Basic/TargetCXXABI.h:116
+///   - static initialization is adjusted to use sinit and sterm functions;
+XL_Clang,
+

daltenty wrote:
> Why the underscore in the name? This is a bit inconsistent with both the LLVM 
> naming convention here and the name as it appears in other sources.
There are various AIX ABI. So to distinguish the one we are implementing, we 
choose `XL` and `Clang` as two parts of the abi name. 
`XL` - not g++;
`Clang` - it's a  ABI implemented in Clang;

And also `XLClang` is misleading because it represents our AIX XL C/C++ 
compiler itself externally.



Comment at: clang/lib/CodeGen/ItaniumCXXABI.cpp:520
+
+class XLClangCXXABI final : public ItaniumCXXABI {
+public:

stevewan wrote:
> The class name here is inconsistent with how our ABI kind was called 
> previously, as David pointed out. Maybe rename the ABI kind `XLClang`? 
I understand you concerns, and please see my replies above.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74015



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

[PATCH] D74372: [OpenMP][IRBuilder] Perform finalization (incl. outlining) late

[Vedant Kumar via Phabricator via cfe-commits]

vsk added a comment.

Thanks :). IIRC `check-clang` is enough to exercise the relevant code path, as 
we get an assert in CodeExtractor without the workaround. (side note: please 
don't take my comments here as blocking, I just wanted to see if we could 
delete some cruft)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74372



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

[PATCH] D74433: [llvm-objdump] Print file format in lowercase to match GNU output.

[Fangrui Song via Phabricator via cfe-commits]

MaskRay accepted this revision.
MaskRay added a comment.
This revision is now accepted and ready to land.

Verified that coff, elf, and mach-o are printed as lower cases. I believe bpf 
is similar.




Comment at: llvm/tools/llvm-objdump/llvm-objdump.cpp:2176
+outs() << ":\tfile format ";
+printLowerCase(O->getFileFormatName(), outs());
+outs() << "\n\n";

`outs() << ":\tfile format " << O->getFileFormatName().lower() << "\n\n";`



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74433



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

[clang] 07f7c00 - [RISCV] Add support for save/restore of callee-saved registers via libcalls

[via cfe-commits]

Author: lewis-revill
Date: 2020-02-11T21:23:03Z
New Revision: 07f7c00208b393296f8f27d6cd3cec2b11d86fd8

URL: 
https://github.com/llvm/llvm-project/commit/07f7c00208b393296f8f27d6cd3cec2b11d86fd8
DIFF: 
https://github.com/llvm/llvm-project/commit/07f7c00208b393296f8f27d6cd3cec2b11d86fd8.diff

LOG: [RISCV] Add support for save/restore of callee-saved registers via libcalls

This patch adds the support required for using the __riscv_save and
__riscv_restore libcalls to implement a size-optimization for prologue
and epilogue code, whereby the spill and restore code of callee-saved
registers is implemented by common functions to reduce code duplication.

Logic is also included to ensure that if both this optimization and
shrink wrapping are enabled then the prologue and epilogue code can be
safely inserted into the basic blocks chosen by shrink wrapping.

Differential Revision: https://reviews.llvm.org/D62686

Added: 
llvm/test/CodeGen/RISCV/saverestore.ll

Modified: 
clang/lib/Driver/ToolChains/Arch/RISCV.cpp
clang/test/Driver/riscv-features.c
llvm/lib/Target/RISCV/RISCV.td
llvm/lib/Target/RISCV/RISCVFrameLowering.cpp
llvm/lib/Target/RISCV/RISCVFrameLowering.h
llvm/lib/Target/RISCV/RISCVMachineFunctionInfo.h
llvm/lib/Target/RISCV/RISCVRegisterInfo.cpp
llvm/lib/Target/RISCV/RISCVRegisterInfo.h
llvm/lib/Target/RISCV/RISCVSubtarget.h
llvm/test/CodeGen/RISCV/shrinkwrap.ll

Removed: 




diff  --git a/clang/lib/Driver/ToolChains/Arch/RISCV.cpp 
b/clang/lib/Driver/ToolChains/Arch/RISCV.cpp
index 5f69fddb5e03..f5764ca5dc06 100644
--- a/clang/lib/Driver/ToolChains/Arch/RISCV.cpp
+++ b/clang/lib/Driver/ToolChains/Arch/RISCV.cpp
@@ -433,12 +433,11 @@ void riscv::getRISCVTargetFeatures(const Driver , const 
llvm::Triple ,
 Features.push_back("-relax");
 
   // GCC Compatibility: -mno-save-restore is default, unless -msave-restore is
-  // specified...
-  if (Args.hasFlag(options::OPT_msave_restore, options::OPT_mno_save_restore, 
false)) {
-// ... but we don't support -msave-restore, so issue a warning.
-D.Diag(diag::warn_drv_clang_unsupported)
-  << Args.getLastArg(options::OPT_msave_restore)->getAsString(Args);
-  }
+  // specified.
+  if (Args.hasFlag(options::OPT_msave_restore, options::OPT_mno_save_restore, 
false))
+Features.push_back("+save-restore");
+  else
+Features.push_back("-save-restore");
 
   // Now add any that the user explicitly requested on the command line,
   // which may override the defaults.

diff  --git a/clang/test/Driver/riscv-features.c 
b/clang/test/Driver/riscv-features.c
index 2f2ae51e4716..8d76e681289b 100644
--- a/clang/test/Driver/riscv-features.c
+++ b/clang/test/Driver/riscv-features.c
@@ -16,9 +16,10 @@
 // RUN: %clang -target riscv32-unknown-elf -### %s -msave-restore 2>&1 | 
FileCheck %s -check-prefix=SAVE-RESTORE
 // RUN: %clang -target riscv32-unknown-elf -### %s -mno-save-restore 2>&1 | 
FileCheck %s -check-prefix=NO-SAVE-RESTORE
 
-// SAVE-RESTORE: warning: the clang compiler does not support '-msave-restore'
-// NO-SAVE-RESTORE-NOT: warning: the clang compiler does not support
-// DEFAULT-NOT: warning: the clang compiler does not support
+// SAVE-RESTORE: "-target-feature" "+save-restore"
+// NO-SAVE-RESTORE: "-target-feature" "-save-restore"
+// DEFAULT: "-target-feature" "-save-restore"
+// DEFAULT-NOT: "-target-feature" "+save-restore"
 
 // RUN: %clang -target riscv32-linux -### %s -fsyntax-only 2>&1 \
 // RUN:   | FileCheck %s -check-prefix=DEFAULT-LINUX

diff  --git a/llvm/lib/Target/RISCV/RISCV.td b/llvm/lib/Target/RISCV/RISCV.td
index 770e883221d1..5f887fdf79ef 100644
--- a/llvm/lib/Target/RISCV/RISCV.td
+++ b/llvm/lib/Target/RISCV/RISCV.td
@@ -82,6 +82,9 @@ foreach i = {1-31} in
 SubtargetFeature<"reserve-x"#i, "UserReservedRegister[RISCV::X"#i#"]",
  "true", "Reserve X"#i>;
 
+def FeatureSaveRestore : SubtargetFeature<"save-restore", "EnableSaveRestore",
+  "true", "Enable save/restore.">;
+
 
//===--===//
 // Named operands for CSR instructions.
 
//===--===//

diff  --git a/llvm/lib/Target/RISCV/RISCVFrameLowering.cpp 
b/llvm/lib/Target/RISCV/RISCVFrameLowering.cpp
index f7cd19cbb8e7..de6e8bf090d4 100644
--- a/llvm/lib/Target/RISCV/RISCVFrameLowering.cpp
+++ b/llvm/lib/Target/RISCV/RISCVFrameLowering.cpp
@@ -23,6 +23,100 @@
 
 using namespace llvm;
 
+// Get the ID of the libcall used for spilling and restoring callee saved
+// registers. The ID is representative of the number of registers saved or
+// restored by the libcall, except it is zero-indexed - ID 0 corresponds to a
+// single register.
+static int getLibCallID(const MachineFunction ,
+const std::vector ) {
+  const auto *RVFI 

[PATCH] D74372: [OpenMP][IRBuilder] Perform finalization (incl. outlining) late

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert added a comment.

In D74372#1870547 , @vsk wrote:

> Can this delete `NeedWorkaroundForOpenMPIRBuilderBug` from 
> llvm/lib/Transforms/Utils/CodeExtractor.cpp?


I didn't know about the workaround but that was the plan. I'll verify and add 
it to the patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74372



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

[PATCH] D72875: [clang][cmake] Include generated rst files in html built by docs-clang-html target

[Dan Liew via Phabricator via cfe-commits]

delcypher requested changes to this revision.
delcypher added inline comments.
This revision now requires changes to proceed.



Comment at: clang/docs/CMakeLists.txt:93
 
+function (gen_rst_file output_file td_option source)
+  get_filename_component(TABLEGEN_INCLUDE_DIR 
"${CMAKE_CURRENT_SOURCE_DIR}/${source}" DIRECTORY)

`gen_rst_file` is a very ambiguous name. At call sites it might not be obvious 
what this function does. Something like `gen_rst_file_from_td` might be more 
helpful.



Comment at: clang/docs/CMakeLists.txt:94
+function (gen_rst_file output_file td_option source)
+  get_filename_component(TABLEGEN_INCLUDE_DIR 
"${CMAKE_CURRENT_SOURCE_DIR}/${source}" DIRECTORY)
+  list(APPEND LLVM_TABLEGEN_FLAGS "-I${TABLEGEN_INCLUDE_DIR}")

You might want to add a check that `${CMAKE_CURRENT_SOURCE_DIR}/${source}` 
exists in this function and error if it does not exist.



Comment at: clang/docs/CMakeLists.txt:97
+  clang_tablegen(${output_file} ${td_option} SOURCE ${source} TARGET 
"gen-${output_file}")
+  add_dependencies(docs-clang-html "gen-${output_file}")
+endfunction()

This `add_dependencies(...)` command is very fragile and is creating an 
implicit coupling between:

* The implementation of `add_sphinx_target()`.
* The declaration of the clang html sphinx target.
* The implementation of `gen_rst_file`.

`gen_rst_file` should probably take an optional argument for the target (in 
this case `docs-clang-html`) to add a dependency to.

This does not completely fix the dependency on `add_sphinx_target` 
implementation but it does mean that `gen_rst_file` isn't also dependent.



Comment at: clang/docs/CMakeLists.txt:104
 if (${SPHINX_OUTPUT_HTML})
-  add_sphinx_target(html clang)
+  add_sphinx_target(html clang SOURCE_DIR ${CMAKE_CURRENT_BINARY_DIR})
+

Minor nit: You might want to quote uses of `${CMAKE_CURRENT_BINARY_DIR}`, 
`${CMAKE_COMMAND}`, and `${CMAKE_CURRENT_SOURCE_DIR}`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72875



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

[PATCH] D74372: [OpenMP][IRBuilder] Perform finalization (incl. outlining) late

[Vedant Kumar via Phabricator via cfe-commits]

vsk added a comment.

Can this delete `NeedWorkaroundForOpenMPIRBuilderBug` from 
llvm/lib/Transforms/Utils/CodeExtractor.cpp?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74372



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

[clang] 5e37fb1 - [NFC] Delete extra white space in a test case.

[Steven Wan via cfe-commits]

Author: Steven Wan
Date: 2020-02-11T16:00:59-05:00
New Revision: 5e37fb1776de2698c10db697e3c479d0e94234e0

URL: 
https://github.com/llvm/llvm-project/commit/5e37fb1776de2698c10db697e3c479d0e94234e0
DIFF: 
https://github.com/llvm/llvm-project/commit/5e37fb1776de2698c10db697e3c479d0e94234e0.diff

LOG: [NFC] Delete extra white space in a test case.

Remove an extra empty line in one of the AIX driver test cases.

Added: 


Modified: 
clang/test/Driver/aix-as.c

Removed: 




diff  --git a/clang/test/Driver/aix-as.c b/clang/test/Driver/aix-as.c
index 4f67d1ba90b7..cb3053f5acd3 100644
--- a/clang/test/Driver/aix-as.c
+++ b/clang/test/Driver/aix-as.c
@@ -23,7 +23,6 @@
 // CHECK-AS64: "-u" 
 // CHECK-AS64: "-many"
 
-
 // Check powerpc-ibm-aix7.1.0.0, 32-bit. -Xassembler  option. 
 // RUN: %clang -no-canonical-prefixes %s -### -c -o %t.o 2>&1 \
 // RUN: -Xassembler -w \



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

[PATCH] D74372: [OpenMP][IRBuilder] Perform finalization (incl. outlining) late

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert marked 2 inline comments as done.
jdoerfert added inline comments.



Comment at: clang/lib/CodeGen/CodeGenFunction.cpp:113
+  if (llvm::OpenMPIRBuilder *OMPBuilder = CGM.getOpenMPIRBuilder())
+OMPBuilder->finalize();
 }

fghanim wrote:
> jdoerfert wrote:
> > fghanim wrote:
> > > Does this mean that `finalize()` is being called twice everytime, here 
> > > and `~OpenMPIRBuilder()`?
> > > if yes, is there a reason why you want that?
> > It's called twice now, not that it matters. The basic rule is, call it at 
> > least once before you finalize the module. I'll remove the destructor for 
> > now to make it explicit only.
> Right now it doesn't matter, I agree. However, if later `finalize()` is 
> modified for any reason, I worry this may introduce problems unintentionally.
> 
> In any case, if you decide to keep only one of them, I prefer to keep the one 
> in `~OpenMPIRBuilder()`. This way the `OMPBuilder` is self contained, so 
> regardless of the frontend using it, we can be certain it's always called at 
> the end. You can indicate explicitly in the description comment for 
> `finalize()` that it is being called in the destructor, if you want.
That doesn't work, otherwise I would not call finalize here. At least it 
doesn't work without extra changes. As it is right now, the OMPBuilder in 
CodeGenModule is deleted *after* the IR is generated and emitted, so any 
changes at destructor time are too late.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74372



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

[PATCH] D74436: Change clang default to -ffp-model=precise

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added inline comments.
Herald added a subscriber: wuzish.



Comment at: clang/docs/UsersManual.rst:1388
 
-   * ``precise`` Disables optimizations that are not value-safe on 
floating-point data, although FP contraction (FMA) is enabled 
(``-ffp-contract=fast``).  This is the default behavior.
* ``strict`` Enables ``-frounding-math`` and 
``-ffp-exception-behavior=strict``, and disables contractions (FMA).  All of 
the ``-ffast-math`` enablements are disabled.

I'm confused. Where in this patch the `This patch establishes the default 
option for -ffp-model to select "precise".` happens? LHS of diff says it is 
already default


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74436



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

[clang] 2d4f80f - [OPENMP50]Full handling of atomic_default_mem_order in requires

[Alexey Bataev via cfe-commits]

Author: Alexey Bataev
Date: 2020-02-11T15:42:34-05:00
New Revision: 2d4f80f78aa5c25f19c396bf85d022d009706936

URL: 
https://github.com/llvm/llvm-project/commit/2d4f80f78aa5c25f19c396bf85d022d009706936
DIFF: 
https://github.com/llvm/llvm-project/commit/2d4f80f78aa5c25f19c396bf85d022d009706936.diff

LOG: [OPENMP50]Full handling of atomic_default_mem_order in requires
directive.

According to OpenMP 5.0, The atomic_default_mem_order clause specifies the 
default memory ordering behavior for atomic constructs that must be provided by 
an implementation. If the default memory ordering is specified as seq_cst, all 
atomic constructs on which memory-order-clause is not specified behave as if 
the seq_cst clause appears. If the default memory ordering is specified as 
relaxed, all atomic constructs on which memory-order-clause is not specified 
behave as if the relaxed clause appears.
If the default memory ordering is specified as acq_rel, atomic constructs on 
which memory-order-clause is not specified behave as if the release clause 
appears if the atomic write or atomic update operation is specified, as if the 
acquire clause appears if the atomic read operation is specified, and as if the 
acq_rel clause appears if the atomic captured update operation is specified.

Added: 
clang/test/OpenMP/requires_acq_rel_codegen.cpp
clang/test/OpenMP/requires_default_atomic_mem_order_messages.cpp
clang/test/OpenMP/requires_relaxed_codegen.cpp
clang/test/OpenMP/requires_seq_cst_codegen.cpp

Modified: 
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/lib/AST/ASTContext.cpp
clang/lib/CodeGen/CGDecl.cpp
clang/lib/CodeGen/CGOpenMPRuntime.cpp
clang/lib/CodeGen/CGOpenMPRuntime.h
clang/lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
clang/lib/CodeGen/CGOpenMPRuntimeNVPTX.h
clang/lib/CodeGen/CGStmtOpenMP.cpp
clang/lib/Parse/ParseOpenMP.cpp
clang/lib/Sema/SemaOpenMP.cpp
clang/lib/Serialization/ASTReaderDecl.cpp
clang/test/OpenMP/requires_target_messages.cpp

Removed: 




diff  --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td 
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index e5714a744692..37f0acf010d6 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -9852,10 +9852,10 @@ def err_omp_requires_clause_redeclaration : Error <
   "Only one %0 clause can appear on a requires directive in a single 
translation unit">;
 def note_omp_requires_previous_clause : Note <
   "%0 clause previously used here">;
-def err_omp_target_before_requires : Error <
-  "target region encountered before requires directive with '%0' clause">;
-def note_omp_requires_encountered_target : Note <
-  "target previously encountered here">;
+def err_omp_directive_before_requires : Error <
+  "'%0' region encountered before requires directive with '%1' clause">;
+def note_omp_requires_encountered_directive : Note <
+  "'%0' previously encountered here">;
 def err_omp_invalid_scope : Error <
   "'#pragma omp %0' directive must appear only in file scope">;
 def note_omp_invalid_length_on_this_ptr_mapping : Note <

diff  --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp
index c80d3948d003..50a0c3d76da2 100644
--- a/clang/lib/AST/ASTContext.cpp
+++ b/clang/lib/AST/ASTContext.cpp
@@ -10041,6 +10041,8 @@ bool ASTContext::DeclMustBeEmitted(const Decl *D) {
 return true;
   else if (isa(D))
 return true;
+  else if (isa(D))
+return true;
   else if (isa(D))
 return !D->getDeclContext()->isDependentContext();
   else if (isa(D))

diff  --git a/clang/lib/CodeGen/CGDecl.cpp b/clang/lib/CodeGen/CGDecl.cpp
index 29462592887a..1767e744bac7 100644
--- a/clang/lib/CodeGen/CGDecl.cpp
+++ b/clang/lib/CodeGen/CGDecl.cpp
@@ -2537,5 +2537,5 @@ void CodeGenModule::EmitOMPDeclareMapper(const 
OMPDeclareMapperDecl *D,
 }
 
 void CodeGenModule::EmitOMPRequiresDecl(const OMPRequiresDecl *D) {
-  getOpenMPRuntime().checkArchForUnifiedAddressing(D);
+  getOpenMPRuntime().processRequiresDirective(D);
 }

diff  --git a/clang/lib/CodeGen/CGOpenMPRuntime.cpp 
b/clang/lib/CodeGen/CGOpenMPRuntime.cpp
index 23d49b23a3b4..c3e2e1e0a5d9 100644
--- a/clang/lib/CodeGen/CGOpenMPRuntime.cpp
+++ b/clang/lib/CodeGen/CGOpenMPRuntime.cpp
@@ -21,6 +21,7 @@
 #include "clang/AST/StmtOpenMP.h"
 #include "clang/AST/StmtVisitor.h"
 #include "clang/Basic/BitmaskEnum.h"
+#include "clang/Basic/OpenMPKinds.h"
 #include "clang/CodeGen/ConstantInitBuilder.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/SetOperations.h"
@@ -30,6 +31,7 @@
 #include "llvm/IR/DerivedTypes.h"
 #include "llvm/IR/GlobalValue.h"
 #include "llvm/IR/Value.h"
+#include "llvm/Support/AtomicOrdering.h"
 #include "llvm/Support/Format.h"
 #include "llvm/Support/raw_ostream.h"
 #include 
@@ -9784,16 +9786,33 @@ void 
CGOpenMPRuntime::adjustTargetSpecificDataForLambdas(
  " Expected 

[PATCH] D74436: Change clang default to -ffp-model=precise

[Melanie Blower via Phabricator via cfe-commits]

mibintc created this revision.
mibintc added reviewers: andrew.w.kaylor, lebedev.ri, rjmccall, sepavloff.
Herald added subscribers: kbarton, nemanjai.
Herald added a project: clang.

This patch establishes the default option for -ffp-model to select "precise".  
Further, -ffp-model=precise also enables -ffp-contract=on (previously 
-ffp-model=precise enabled -ffp-contract=fast).  This is a follow-up to Andy 
Kaylor's comments in the llvm-dev discussion "Floating Point semantic modes".  
From the same email thread, I put Andy's distillation of floating point options 
and floating point modes into UsersManual.rst


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D74436

Files:
  clang/docs/UsersManual.rst
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/CodeGen/ppc-emmintrin.c
  clang/test/CodeGen/ppc-xmmintrin.c
  clang/test/Driver/fp-model.c

Index: clang/test/Driver/fp-model.c
===
--- clang/test/Driver/fp-model.c
+++ clang/test/Driver/fp-model.c
@@ -27,9 +27,9 @@
 // RUN:   | FileCheck --check-prefix=WARN5 %s
 // WARN5: warning: overriding '-ffp-model=strict' option with '-ffp-contract=fast' [-Woverriding-t-option]
 
-// RUN: %clang -### -ffp-model=strict -ffp-contract=off -c %s 2>&1 \
+// RUN: %clang -### -ffp-model=strict -ffp-contract=fast -c %s 2>&1 \
 // RUN:   | FileCheck --check-prefix=WARN6 %s
-// WARN6: warning: overriding '-ffp-model=strict' option with '-ffp-contract=off' [-Woverriding-t-option]
+// WARN6: warning: overriding '-ffp-model=strict' option with '-ffp-contract=fast' [-Woverriding-t-option]
 
 // RUN: %clang -### -ffp-model=strict -ffp-contract=on -c %s 2>&1 \
 // RUN:   | FileCheck --check-prefix=WARN7 %s
@@ -100,13 +100,14 @@
 // RUN: %clang -### -nostdinc -ffp-model=precise -c %s 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-FPM-PRECISE %s
 // CHECK-FPM-PRECISE: "-cc1"
-// CHECK-FPM-PRECISE: "-ffp-contract=fast"
+// CHECK-FPM-PRECISE: "-ffp-contract=on"
 // CHECK-FPM-PRECISE: "-fno-rounding-math"
 
 // RUN: %clang -### -nostdinc -ffp-model=strict -c %s 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-FPM-STRICT %s
 // CHECK-FPM-STRICT: "-cc1"
 // CHECK-FPM-STRICT: "-ftrapping-math"
+// CHECK-FPM-STRICT: "-ffp-contract=off"
 // CHECK-FPM-STRICT: "-frounding-math"
 // CHECK-FPM-STRICT: "-ffp-exception-behavior=strict"
 
Index: clang/test/CodeGen/ppc-xmmintrin.c
===
--- clang/test/CodeGen/ppc-xmmintrin.c
+++ clang/test/CodeGen/ppc-xmmintrin.c
@@ -2,9 +2,9 @@
 // REQUIRES: powerpc-registered-target
 
 // RUN: %clang -S -emit-llvm -target powerpc64-unknown-linux-gnu -mcpu=pwr8 -ffreestanding -DNO_WARN_X86_INTRINSICS %s \
-// RUN:   -fno-discard-value-names -mllvm -disable-llvm-optzns -o - | llvm-cxxfilt -n | FileCheck %s --check-prefixes=CHECK,CHECK-BE
+// RUN:   -ffp-contract=off -fno-discard-value-names -mllvm -disable-llvm-optzns -o - | llvm-cxxfilt -n | FileCheck %s --check-prefixes=CHECK,CHECK-BE
 // RUN: %clang -S -emit-llvm -target powerpc64le-unknown-linux-gnu -mcpu=pwr8 -ffreestanding -DNO_WARN_X86_INTRINSICS %s \
-// RUN:   -fno-discard-value-names -mllvm -disable-llvm-optzns -o - | llvm-cxxfilt -n | FileCheck %s --check-prefixes=CHECK,CHECK-LE
+// RUN:   -ffp-contract=off -fno-discard-value-names -mllvm -disable-llvm-optzns -o - | llvm-cxxfilt -n | FileCheck %s --check-prefixes=CHECK,CHECK-LE
 
 #include 
 
Index: clang/test/CodeGen/ppc-emmintrin.c
===
--- clang/test/CodeGen/ppc-emmintrin.c
+++ clang/test/CodeGen/ppc-emmintrin.c
@@ -2,9 +2,9 @@
 // REQUIRES: powerpc-registered-target
 
 // RUN: %clang -S -emit-llvm -target powerpc64-unknown-linux-gnu -mcpu=pwr8 -ffreestanding -DNO_WARN_X86_INTRINSICS %s \
-// RUN:  -fno-discard-value-names -mllvm -disable-llvm-optzns -o - | llvm-cxxfilt -n | FileCheck %s --check-prefixes=CHECK,CHECK-BE
+// RUN:  -ffp-contract=off -fno-discard-value-names -mllvm -disable-llvm-optzns -o - | llvm-cxxfilt -n | FileCheck %s --check-prefixes=CHECK,CHECK-BE
 // RUN: %clang -S -emit-llvm -target powerpc64le-unknown-linux-gnu -mcpu=pwr8 -ffreestanding -DNO_WARN_X86_INTRINSICS %s \
-// RUN:   -fno-discard-value-names -mllvm -disable-llvm-optzns -o - | llvm-cxxfilt -n | FileCheck %s --check-prefixes=CHECK,CHECK-LE
+// RUN:   -ffp-contract=off -fno-discard-value-names -mllvm -disable-llvm-optzns -o - | llvm-cxxfilt -n | FileCheck %s --check-prefixes=CHECK,CHECK-LE
 
 // CHECK-BE-DAG: @_mm_movemask_pd.perm_mask = internal constant <4 x i32> , align 16
 // CHECK-BE-DAG: @_mm_shuffle_epi32.permute_selectors = internal constant [4 x i32] [i32 66051, i32 67438087, i32 134810123, i32 202182159], align 4
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -2520,10 +2520,9 @@
 
   llvm::DenormalMode 

[PATCH] D72875: [clang][cmake] Include generated rst files in html built by docs-clang-html target

[Tom Stellard via Phabricator via cfe-commits]

tstellar added a comment.

Any other commits before I commit this?




Comment at: llvm/cmake/modules/AddSphinxTarget.cmake:33
+  if (NOT ARG_SOURCE_DIR)
+set(ARG_SOURCE_DIR ${CMAKE_CURRENT_SOURCE_DIR})
+  endif()

delcypher wrote:
> @tstellar I'm not 100% sure about this but I think you probably want 
> `${CMAKE_CURRENT_SOURCE_DIR}` quoted. If it's not then if it contains spaces 
> I think you might make `ARG_SOURCE_DIR` a list. Then we you go to print it 
> you'll probably end up with list separators (i.e. `;`) in the command you 
> pass the the sphinx binary.
> 
> I've not tested this though so I could be wrong.
There are quotes used when setting variables above, so I went ahead and changed 
this to match.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72875



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

[PATCH] D72811: [WIP][OPENMP5.0] allow lvalue for motion clause

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.



Comment at: clang/lib/Sema/SemaOpenMP.cpp:15293
+  //  TypeDecorCnt for B: 0
+  if (Depth <= TypeDecorCnt) {
+RelevantExpr = cast(DRE);

cchen wrote:
> ABataev wrote:
> > The check is really bad. If you want to handle something like `*(B+x)` 
> > better to treat, say, `B` as the base and `x` as the index and, thus, treat 
> > the whole expression as something like `B[x]`
> I don't understand how to do this, is there any sample code that I can learn 
> from? Thanks
Not sure we have something like this, need to invent something new if we want 
to support lvalues in full. Or discard it as unsupported if we're unable to 
support it properly.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72811



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

[PATCH] D72841: [RFC] Add support for pragma float_control, to control precision and exception behavior at the source level

[Melanie Blower via Phabricator via cfe-commits]

mibintc added a comment.

In D72841#1869931 , @lebedev.ri wrote:

> I would think `contract` change can be separated from the rest of the 
> changes, and therefore should be a separate review (to reduce noise)?


I split off that change to https://reviews.llvm.org/D74436 and added you as 
reviewer, thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72841



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

[PATCH] D72811: [WIP][OPENMP5.0] allow lvalue for motion clause

[Chi Chun Chen via Phabricator via cfe-commits]

cchen marked an inline comment as done.
cchen added inline comments.



Comment at: clang/lib/Sema/SemaOpenMP.cpp:15293
+  //  TypeDecorCnt for B: 0
+  if (Depth <= TypeDecorCnt) {
+RelevantExpr = cast(DRE);

ABataev wrote:
> The check is really bad. If you want to handle something like `*(B+x)` better 
> to treat, say, `B` as the base and `x` as the index and, thus, treat the 
> whole expression as something like `B[x]`
I don't understand how to do this, is there any sample code that I can learn 
from? Thanks


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72811



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

[PATCH] D73629: [analyzer] vfork checker: allow execve after vfork

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

Let's still add the test. It'll make sure that you understand exactly what 
you're doing and that there aren't more aspects to this problem. More tests 
never hurt and it's a local culture to have at least some tests for every 
commit that changes the behavior. Our tests are also very cheap to add.


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

https://reviews.llvm.org/D73629



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

[PATCH] D68049: Propeller: Clang options for basic block sections

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

In D68049#1870401 , @tmsriram wrote:

> In D68049#1870094 , @tmsriram wrote:
>
> > In D68049#1868623 , @MaskRay wrote:
> >
> > > > In D68049#1865967 , @MaskRay 
> > > > wrote:
> > > >  If you don't mind, I can push a Diff to this Differential which will 
> > > > address these review comments.
> > >
> > > I can't because I can't figure out the patch relationship...
> > >
> > > First, this patch does not build on its own. I try applying D68063 
> > >  first, then this patch. It still does 
> > > not compile..
> >
>
>
> This should work now.  Please apply D68063  
> first and then this one. Thanks!


Please can you specify all those patch relations by marking patches as 
parent/child in phab ui?


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

https://reviews.llvm.org/D68049



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

[PATCH] D72811: [WIP][OPENMP5.0] allow lvalue for motion clause

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added a comment.

I would suggest starting with the reworking of the existing implementation with 
StmtVisitor class and after that step-by-step extend the functionality of the 
visitor with handling other kinds а expressions.




Comment at: clang/lib/Sema/SemaOpenMP.cpp:15274-15276
+  const std::string TypeStr = PT->getCanonicalTypeInternal().getAsString();
+  size_t TypeDecorCnt = std::count_if(TypeStr.begin(), TypeStr.end(),
+ [](char c) { return c == '*' || c == '['; });

Again, this definitely should be reworked.



Comment at: clang/lib/Sema/SemaOpenMP.cpp:15293
+  //  TypeDecorCnt for B: 0
+  if (Depth <= TypeDecorCnt) {
+RelevantExpr = cast(DRE);

The check is really bad. If you want to handle something like `*(B+x)` better 
to treat, say, `B` as the base and `x` as the index and, thus, treat the whole 
expression as something like `B[x]`



Comment at: clang/lib/Sema/SemaOpenMP.cpp:15303
+  bool VisitMemberExpr(MemberExpr *ME) {
+Expr *E = cast(ME);
+Expr *BaseE = ME->getBase()->IgnoreParenImpCasts();

Why do you need this?



Comment at: clang/lib/Sema/SemaOpenMP.cpp:15338
+return false;
+  return Visit(E->IgnoreParenImpCasts());;
+}

Do not call `IgnoreParenImpCasts()` here, just `Visit(E)`. Also, I assume, it 
may lead to infinite recursion.



Comment at: clang/lib/Sema/SemaOpenMP.cpp:15356
   }
+  return Visit(E->IgnoreParenImpCasts());;
+}

Same here


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72811



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

[PATCH] D72811: [WIP][OPENMP5.0] allow lvalue for motion clause

[Chi Chun Chen via Phabricator via cfe-commits]

cchen marked an inline comment as done.
cchen added a comment.

> Quoted Text






Comment at: clang/lib/Sema/SemaOpenMP.cpp:15554
+LocatorChecker Checker;
+if (Checker.Visit(OrigExpr)) {
+  llvm::copy(Checker.getComponents(),

ABataev wrote:
> cchen wrote:
> > ABataev wrote:
> > > cchen wrote:
> > > > ABataev wrote:
> > > > > General question about several cases. How we're going to support them?
> > > > > 1. (a ? b : c). 
> > > > > 2. __builtin_choose_expr(a, b, c).
> > > > > 3. a = b.
> > > > > 4. a?:b
> > > > > 5. __builtin_convertvector(x, ty)
> > > > > 6. (int&)a
> > > > > 7. v.xy, where v is an extended vector
> > > > > 8. a.b, where b is a bitfield
> > > > > 9. __builtin_bit_cast(v, ty)
> > > > > 10. const_cast(a)
> > > > > 11. dynamic_cast(a)
> > > > > 12. reinterpret_cast
> > > > > 13. static_cast
> > > > > 14. typeid() (also is an lvalue).
> > > > > 15. __uuidof(*comPtr)
> > > > > 16. lambda calls
> > > > > 17. User defined literals
> > > > > 
> > > > I think we could first evaluate the lvalue, and then pass the result of 
> > > > `` to device? 
> > > What do you mean when you say `evaluate lvalue`? In veneral, it can be 
> > > evaluated at the runtime. Do you propose to implement dyic translation? 
> > > It will definetely slow down the execution on the device.
> > I mean evaluate lvalue before sending  to device. For example:
> > ```
> > int a, b;
> > b = 5;
> > #pragma omp target map(a = b) // assign b to a before sending `&(a=b)` to 
> > device
> > {
> >a++;
> > }
> > ```
> > Therefore, the above example have the same semantics as this:
> > ```
> > int a, b;
> > b = 5;
> > int  = (a=b)
> > #pragma omp target map(c)
> > {
> >a++;
> > }
> > ```
> Sure, we do this already, generally speaking. The main problem here is how to 
> map the address and associate it with address on the device. We do it at the 
> compile time, support of general lvalues requires runtime translation + 
> special instrumentation of the device code for address translation.
  - (a ? b : c).
- Error out for now, not sure what to for this one
  - __builtin_choose_expr(a, b, c).
- Allow this one since we know what to send to device at compile time
  - a = b.
   - Sema accept this one, but need more work for codegen I think (Clang not 
emit the a=b ir if "a=b" inside map/motion clause)
  - a?:b
- What's the name of this kind of expression? I don't know what to do for 
this one
  - __builtin_convertvector(x, ty)
- not sure
  - v.xy, where v is an extended vector
- not sure
  - a.b, where b is a bitfield
- Error out for this one since bitfield is not addressable
  - __builtin_bit_cast(v, ty)
- Error out for this one since bitfield is not addressable
  - casts
- Error out if the expression inside the cast is not lvalue
  - typeid() (also is an lvalue).
- I guess we can support this just like normal variable?
  - __uuidof(*comPtr)
- not sure
  - lambda calls
- OpenMP 5.0 spec has rules for lambda. Now not error out for sema but need 
more work for codegen
  - User defined literals
 - error out since not addressable



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72811



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

[PATCH] D73052: [clang-tidy] RenamerClangTidy now renames dependent member expr when the member can be resolved

[Nathan James via Phabricator via cfe-commits]

njames93 updated this revision to Diff 243953.
njames93 added a comment.

- Simplified member expr restrictions


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73052

Files:
  clang-tools-extra/clang-tidy/bugprone/ReservedIdentifierCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/ReservedIdentifierCheck.h
  clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
  clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.h
  clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp
  clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/readability-identifier-naming.rst
  
clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-member-decl-usage.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-member-decl-usage.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-member-decl-usage.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-member-decl-usage.cpp
@@ -1,7 +1,9 @@
 // RUN: %check_clang_tidy %s readability-identifier-naming %t -- \
 // RUN:   -config='{CheckOptions: [ \
 // RUN: {key: readability-identifier-naming.MemberCase, value: CamelCase}, \
-// RUN: {key: readability-identifier-naming.ParameterCase, value: CamelCase} \
+// RUN: {key: readability-identifier-naming.ParameterCase, value: CamelCase}, \
+// RUN: {key: readability-identifier-naming.MethodCase, value: camelBack}, \
+// RUN: {key: readability-identifier-naming.AggressiveDependentMemberLookup, value: 1} \
 // RUN:  ]}' -- -fno-delayed-template-parsing
 
 int set_up(int);
@@ -63,32 +65,23 @@
   // CHECK-FIXES: {{^}}  int getBar2() const { return this->BarBaz; } // comment-9
 };
 
-TempTest x; //force an instantiation
-
-int blah() {
-  return x.getBar2(); // gotta have a reference to the getBar2 so that the
-  // compiler will generate the function and resolve
-  // the DependantScopeMemberExpr
-}
-
 namespace Bug41122 {
 namespace std {
 
 // for this example we aren't bothered about how std::vector is treated
-template  //NOLINT
-class vector { //NOLINT
-public:
-  void push_back(bool); //NOLINT
-  void pop_back(); //NOLINT
-}; //NOLINT
-}; // namespace std
+template// NOLINT
+struct vector { // NOLINT
+  void push_back(bool); // NOLINT
+  void pop_back();  // NOLINT
+};  // NOLINT
+};  // namespace std
 
-class Foo { 
+class Foo {
   std::vector 
   // CHECK-MESSAGES: :[[@LINE-1]]:22: warning: invalid case style for member 'stack' [readability-identifier-naming]
 public:
   Foo(std::vector )
-  // CHECK-MESSAGES: :[[@LINE-1]]:26: warning: invalid case style for parameter 'stack' [readability-identifier-naming]
+  // CHECK-MESSAGES: :[[@LINE-1]]:26: warning: invalid case style for parameter 'stack' [readability-identifier-naming]
   // CHECK-FIXES: {{^}}  Foo(std::vector )
   : stack(stack) {
 // CHECK-FIXES: {{^}}  : Stack(Stack) {
@@ -134,4 +127,92 @@
 void foo() {
   Container container;
 }
-}; // namespace CtorInits
+} // namespace CtorInits
+
+namespace resolved_dependance {
+template 
+struct A0 {
+  int value;
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: invalid case style for member 'value'
+  A0 =(const A0 ) {
+value = Other.value;   // A0
+this->value = Other.value; // A0
+// CHECK-FIXES:  {{^}}Value = Other.Value;   // A0
+// CHECK-FIXES-NEXT: {{^}}this->Value = Other.Value; // A0
+return *this;
+  }
+  void outOfLineReset();
+};
+
+template 
+void A0::outOfLineReset() {
+  this->value -= value; // A0
+  // CHECK-FIXES: {{^}}  this->Value -= Value; // A0
+}
+
+template 
+struct A1 {
+  int value; // A1
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: invalid case style for member 'value'
+  // CHECK-FIXES: {{^}}  int Value; // A1
+  int GetValue() const { return value; } // A1
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: invalid case style for method 'GetValue'
+  // CHECK-FIXES {{^}}  int getValue() const { return Value; } // A1
+  void SetValue(int Value) { this->value = Value; } // A1
+  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: invalid case style for method 'SetValue'
+  // CHECK-FIXES {{^}}  void setValue(int Value) { this->Value = Value; } // A1
+  A1 =(const A1 ) {
+this->SetValue(Other.GetValue()); // A1
+this->value = Other.value;// A1
+// CHECK-FIXES:  {{^}}this->setValue(Other.getValue()); // A1
+// CHECK-FIXES-NEXT: {{^}}this->Value = Other.Value;// A1
+return *this;
+  }
+  void outOfLineReset();
+};
+
+template 
+void A1::outOfLineReset() {
+  this->value -= value; // A1
+  // CHECK-FIXES: {{^}}  this->Value -= Value; // 

[PATCH] D68049: Propeller: Clang options for basic block sections

[Sriraman Tallam via Phabricator via cfe-commits]

tmsriram added a comment.

>>> I think the patch series should probably be structured this way:
>>> 
>>> 1. LLVM CodeGen: enables basic block sections.
>>> 2. clang Driver/Frontend/CodeGen: pass basic block sections options to LLVM.
>>> 3. LLVM CodeGen: which enables the rest of Propeller options.
>>> 4. lld: a file similar to lld/ELF/LTO.cpp . It should be a thin wrapper of 
>>> Propeller features. It should not do hacky diassembly tasks.
>>> 5. clang Driver/Frontend/CodeGen: passes compiler/linker options to 3 and 4
>>> 
>>>   Making 1 and 2 separate can help move forward the patch series. 1 and 2 
>>> should not reference `llvm::propeller`.

It is now structured like how you mentioned above.  1) corresponds to D68063 
 and D73674 . 
 2) corresponds to this patch, D68049 .   
These patches do not reference "propeller" anywhere and are only about basic 
block sections and labels.

We will address Eli's comments in the LLVM patches.  We will also make 3), 4) 
and 5) like how you wanted.  Further, D68065  
is about LLD support for basic block sections and will not reference 
"propeller".


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

https://reviews.llvm.org/D68049



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

[PATCH] D72811: [WIP][OPENMP5.0] allow lvalue for motion clause

[Chi Chun Chen via Phabricator via cfe-commits]

cchen updated this revision to Diff 243950.
cchen added a comment.

1. Refactor based on feedback and use the visitor class to integrate the 
analysis

in checkMapClauseExpressionBase.

2. Handle more lvalue cases


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72811

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/CodeGen/CGOpenMPRuntime.cpp
  clang/lib/Sema/SemaOpenMP.cpp
  clang/test/OpenMP/target_map_messages.cpp
  clang/test/OpenMP/target_teams_map_messages.cpp
  clang/test/OpenMP/target_update_codegen.cpp
  clang/test/OpenMP/target_update_from_messages.cpp
  clang/test/OpenMP/target_update_to_messages.cpp

Index: clang/test/OpenMP/target_update_to_messages.cpp
===
--- clang/test/OpenMP/target_update_to_messages.cpp
+++ clang/test/OpenMP/target_update_to_messages.cpp
@@ -1,6 +1,12 @@
-// RUN: %clang_cc1 -verify -fopenmp -ferror-limit 100 %s -Wno-openmp-mapping -Wuninitialized
+// RUN: %clang_cc1 -verify=expected,le45 -fopenmp -ferror-limit 100 %s -Wno-openmp-mapping -Wuninitialized
+// RUN: %clang_cc1 -verify=expected,le45 -fopenmp -fopenmp-version=40 -ferror-limit 100 %s -Wno-openmp-mapping -Wuninitialized
+// RUN: %clang_cc1 -verify=expected,le45 -fopenmp -fopenmp-version=45 -ferror-limit 100 %s -Wno-openmp-mapping -Wuninitialized
+// RUN: %clang_cc1 -verify=expected,le50 -fopenmp -fopenmp-version=50 -ferror-limit 100 %s -Wno-openmp-mapping -Wuninitialized
 
-// RUN: %clang_cc1 -verify -fopenmp-simd -ferror-limit 100 %s -Wno-openmp-mapping -Wuninitialized
+// RUN: %clang_cc1 -verify=expected,le45 -fopenmp-simd -ferror-limit 100 %s -Wno-openmp-mapping -Wuninitialized
+// RUN: %clang_cc1 -verify=expected,le45 -fopenmp-simd -fopenmp-version=40 -ferror-limit 100 %s -Wno-openmp-mapping -Wuninitialized
+// RUN: %clang_cc1 -verify=expected,le45 -fopenmp-simd -fopenmp-version=45 -ferror-limit 100 %s -Wno-openmp-mapping -Wuninitialized
+// RUN: %clang_cc1 -verify=expected,le50 -fopenmp-simd -fopenmp-version=50 -ferror-limit 100 %s -Wno-openmp-mapping -Wuninitialized
 
 void foo() {
 }
@@ -59,6 +65,29 @@
   double *p;
   unsigned bfa : 4;
 };
+struct S8 {
+  int *ptr;
+  int a;
+  struct S7* S;
+  void foo() {
+#pragma omp target update to(*this) // le45-error {{expected expression containing only member accesses and/or array sections based on named variables}} le45-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
+{}
+#pragma omp target update to(*(&(*this))) // le45-error {{expected expression containing only member accesses and/or array sections based on named variables}} le45-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
+#pragma omp target update to(*(this->ptr)) // le45-error {{expected expression containing only member accesses and/or array sections based on named variables}} le45-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
+#pragma omp target update to(*(&(*(this->S->i+this->S->p // le45-error {{expected expression containing only member accesses and/or array sections based on named variables}} le45-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
+#pragma omp target update to(*(&(*(this->S->i+this->S->s6[0].pp // le45-error {{expected expression containing only member accesses and/or array sections based on named variables}} le45-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
+#pragma omp target update to(*(&(*(a+this->ptr // le45-error {{expected expression containing only member accesses and/or array sections based on named variables}} le45-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
+#pragma omp target update to(*(&(*(*(this->ptr)+a+this->ptr // le45-error {{expected expression containing only member accesses and/or array sections based on named variables}} le45-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
+{}
+  }
+};
+
+struct Base {
+  virtual ~Base() {}
+};
+struct Derived: Base {
+  virtual void name() {}
+};
 
 S3 h;
 #pragma omp threadprivate(h) // expected-note 2 {{defined as threadprivate or thread local}}
@@ -89,12 +118,12 @@
 #pragma omp target update to(x)
 #pragma omp target update to(t[:I])
 #pragma omp target update to(T) // expected-error {{'T' does not refer to a value}} expected-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
-#pragma omp target update to(I) // expected-error 2 {{expected expression containing only member accesses and/or array sections based on named variables}}
+#pragma omp target update to(I) // le50-error 2 {{expected lvalue with no 

[PATCH] D72218: [clang-tidy] new altera kernel name restriction check

[Frank Derry Wanye via Phabricator via cfe-commits]

ffrankies updated this revision to Diff 243945.
ffrankies marked 9 inline comments as done.
ffrankies edited the summary of this revision.
ffrankies added a comment.

Implemented changes requested by @Eugene.Zelenko:

- Added empty lines around namespace block
- Fixed use of auto keyword
- Fixed formatting in documentation
- Added dependency on previous revision (D66564 
) to the Summary.


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

https://reviews.llvm.org/D72218

Files:
  clang-tidy/CMakeLists.txt
  clang-tidy/ClangTidyForceLinker.h
  clang-tidy/altera/AlteraTidyModule.cpp
  clang-tidy/altera/CMakeLists.txt
  clang-tidy/altera/KernelNameRestrictionCheck.cpp
  clang-tidy/altera/KernelNameRestrictionCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/altera-kernel-name-restriction.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/index.rst
  test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/KERNEL.cl
  test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/VHDL.cl
  test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/Verilog.cl
  test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/kernel.cl
  test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/kernel.h
  
test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/other_Verilog.cl
  
test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/otherdir/vhdl.cl
  test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/otherthing.cl
  
test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/some/dir/kernel.cl
  test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/some_kernel.cl
  
test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/somedir/verilog.cl
  test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/thing.h
  test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/vERILOG.cl
  test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/verilog.h
  test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/vhdl.CL
  test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/vhdl.h
  
test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/vhdl_number_two.cl
  test/clang-tidy/checkers/altera-kernel-name-restriction.cpp

Index: test/clang-tidy/checkers/altera-kernel-name-restriction.cpp
===
--- /dev/null
+++ test/clang-tidy/checkers/altera-kernel-name-restriction.cpp
@@ -0,0 +1,42 @@
+// RUN: %check_clang_tidy %s altera-kernel-name-restriction %t -- -- -I%S/Inputs/altera-kernel-name-restriction
+
+// These are the banned kernel filenames, and should trigger warnings
+#include "kernel.cl"
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: The imported kernel source file is named 'kernel.cl','Verilog.cl', or 'VHDL.cl', which could cause compilation errors. [altera-kernel-name-restriction]
+#include "Verilog.cl"
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: The imported kernel source file is named 'kernel.cl','Verilog.cl', or 'VHDL.cl', which could cause compilation errors. [altera-kernel-name-restriction]
+#include "VHDL.cl"
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: The imported kernel source file is named 'kernel.cl','Verilog.cl', or 'VHDL.cl', which could cause compilation errors. [altera-kernel-name-restriction]
+
+// The warning should be triggered regardless of capitalization
+#include "KERNEL.cl"
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: The imported kernel source file is named 'kernel.cl','Verilog.cl', or 'VHDL.cl', which could cause compilation errors. [altera-kernel-name-restriction]
+#include "vERILOG.cl"
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: The imported kernel source file is named 'kernel.cl','Verilog.cl', or 'VHDL.cl', which could cause compilation errors. [altera-kernel-name-restriction]
+#include "vhdl.CL"
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: The imported kernel source file is named 'kernel.cl','Verilog.cl', or 'VHDL.cl', which could cause compilation errors. [altera-kernel-name-restriction]
+
+// The warning should be triggered if the names are within a directory
+#include "some/dir/kernel.cl"
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: The imported kernel source file is named 'kernel.cl','Verilog.cl', or 'VHDL.cl', which could cause compilation errors. [altera-kernel-name-restriction]
+#include "somedir/verilog.cl"
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: The imported kernel source file is named 'kernel.cl','Verilog.cl', or 'VHDL.cl', which could cause compilation errors. [altera-kernel-name-restriction]
+#include "otherdir/vhdl.cl"
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: The imported kernel source file is named 'kernel.cl','Verilog.cl', or 'VHDL.cl', which could cause compilation errors. [altera-kernel-name-restriction]
+
+// There are no FIX-ITs for the altera-kernel-name-restriction lint check
+
+// The following include directives shouldn't trigger the 

[PATCH] D74372: [OpenMP][IRBuilder] Perform finalization (incl. outlining) late

[Fady Ghanim via Phabricator via cfe-commits]

fghanim added inline comments.



Comment at: clang/lib/CodeGen/CodeGenFunction.cpp:113
+  if (llvm::OpenMPIRBuilder *OMPBuilder = CGM.getOpenMPIRBuilder())
+OMPBuilder->finalize();
 }

jdoerfert wrote:
> fghanim wrote:
> > Does this mean that `finalize()` is being called twice everytime, here and 
> > `~OpenMPIRBuilder()`?
> > if yes, is there a reason why you want that?
> It's called twice now, not that it matters. The basic rule is, call it at 
> least once before you finalize the module. I'll remove the destructor for now 
> to make it explicit only.
Right now it doesn't matter, I agree. However, if later `finalize()` is 
modified for any reason, I worry this may introduce problems unintentionally.

In any case, if you decide to keep only one of them, I prefer to keep the one 
in `~OpenMPIRBuilder()`. This way the `OMPBuilder` is self contained, so 
regardless of the frontend using it, we can be certain it's always called at 
the end. You can indicate explicitly in the description comment for 
`finalize()` that it is being called in the destructor, if you want.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74372



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

[PATCH] D68049: Propeller: Clang options for basic block sections

[Sriraman Tallam via Phabricator via cfe-commits]

tmsriram added a comment.

In D68049#1870094 , @tmsriram wrote:

> In D68049#1868623 , @MaskRay wrote:
>
> > > In D68049#1865967 , @MaskRay 
> > > wrote:
> > >  If you don't mind, I can push a Diff to this Differential which will 
> > > address these review comments.
> >
> > I can't because I can't figure out the patch relationship...
> >
> > First, this patch does not build on its own. I try applying D68063 
> >  first, then this patch. It still does not 
> > compile..
>


This should work now.  Please apply D68063  
first and then this one. Thanks!

> Weird, getBBSectionsList is defined by D68063 
> .  Let me try doing that again.  I will also 
> address the rest of your comments.
> 
>> 
>> 
>>   clang/lib/CodeGen/BackendUtil.cpp:484:11: error: no member named 
>> 'propeller' in namespace 'llvm'
>>   llvm::propeller::getBBSectionsList(CodeGenOpts.BBSections,
>> 
>> 
>> Chatted with shenhan and xur offline. I tend to agree that 
>> -fbasicblock-sections=label can improve profile accuracy. It'd be nice to 
>> make that feature separate,
>>  even if there is still a debate on whether the rest of Propeller is done in 
>> a maintainable way.
>> 
>> I think the patch series should probably be structured this way:
>> 
>> 1. LLVM CodeGen: enables basic block sections.
>> 2. clang Driver/Frontend/CodeGen: pass basic block sections options to LLVM.
>> 3. LLVM CodeGen: which enables the rest of Propeller options.
>> 4. lld: a file similar to lld/ELF/LTO.cpp . It should be a thin wrapper of 
>> Propeller features. It should not do hacky diassembly tasks.
>> 5. clang Driver/Frontend/CodeGen: passes compiler/linker options to 3 and 4
>> 
>>   Making 1 and 2 separate can help move forward the patch series. 1 and 2 
>> should not reference `llvm::propeller`.




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

https://reviews.llvm.org/D68049



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

[PATCH] D68049: Propeller: Clang options for basic block sections

[Sriraman Tallam via Phabricator via cfe-commits]

tmsriram updated this revision to Diff 243947.
tmsriram added a comment.

Remove usage of "propeller". Fix header inclusion.


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

https://reviews.llvm.org/D68049

Files:
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Basic/CodeGenOptions.h
  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/basicblock-sections.c

Index: clang/test/CodeGen/basicblock-sections.c
===
--- /dev/null
+++ clang/test/CodeGen/basicblock-sections.c
@@ -0,0 +1,47 @@
+// REQUIRES: x86-registered-target
+
+// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -S -o - < %s | FileCheck %s --check-prefix=PLAIN
+// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -S -fbasicblock-sections=all -fbasicblock-sections=none -o - < %s | FileCheck %s --check-prefix=PLAIN
+
+// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -S -fbasicblock-sections=labels -o - < %s | FileCheck %s --check-prefix=BB_LABELS
+// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -S -fbasicblock-sections=all -o - < %s | FileCheck %s --check-prefix=BB_WORLD --check-prefix=BB_ALL
+// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -S -fbasicblock-sections=%S/basicblock-sections.funcnames -o - < %s | FileCheck %s --check-prefix=BB_WORLD --check-prefix=BB_LIST
+// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -S -fbasicblock-sections=all -funique-bb-section-names -o - < %s | FileCheck %s --check-prefix=UNIQUE
+
+int world(int a) {
+  if (a > 10)
+return 10;
+  else if (a > 5)
+return 5;
+  else
+return 0;
+}
+
+int another(int a) {
+  if (a > 10)
+return 20;
+  return 0;
+}
+
+// PLAIN-NOT: section
+// PLAIN: world
+//
+// BB_LABELS-NOT: section
+// BB_LABELS: world
+// BB_LABELS-LABEL: a.BB.world
+// BB_LABELS-LABEL: aa.BB.world
+// BB_LABEL-LABEL: a.BB.another
+//
+// BB_WORLD: .section .text.world,"ax",@progbits
+// BB_WORLD: world
+// BB_WORLD: .section .text.world,"ax",@progbits,unique
+// BB_WORLD: a.BB.world
+// BB_WORLD: .section .text.another,"ax",@progbits
+// BB_ALL: .section .text.another,"ax",@progbits,unique
+// BB_ALL: a.BB.another
+// BB_LIST-NOT: .section .text.another,"ax",@progbits,unique
+// BB_LIST: another
+// BB_LIST-NOT: a.BB.another
+//
+// UNIQUE: .section .text.world.a.BB.world
+// UNIQUE: .section .text.another.a.BB.another
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -962,10 +962,24 @@
   Opts.TrapFuncName = std::string(Args.getLastArgValue(OPT_ftrap_function_EQ));
   Opts.UseInitArray = !Args.hasArg(OPT_fno_use_init_array);
 
-  Opts.FunctionSections = Args.hasArg(OPT_ffunction_sections);
+  Opts.BBSections =
+  std::string(Args.getLastArgValue(OPT_fbasicblock_sections_EQ, "none"));
+  if (Opts.BBSections != "all" && Opts.BBSections != "labels" &&
+  Opts.BBSections != "none" && !llvm::sys::fs::exists(Opts.BBSections)) {
+Diags.Report(diag::err_drv_invalid_value)
+<< Args.getLastArg(OPT_fbasicblock_sections_EQ)->getAsString(Args)
+<< Opts.BBSections;
+  }
+
+  // Basic Block Sections implies Function Sections.
+  Opts.FunctionSections =
+  Args.hasArg(OPT_ffunction_sections) ||
+  (Opts.BBSections != "none" && Opts.BBSections != "labels");
+
   Opts.DataSections = Args.hasArg(OPT_fdata_sections);
   Opts.StackSizeSection = Args.hasArg(OPT_fstack_size_section);
   Opts.UniqueSectionNames = !Args.hasArg(OPT_fno_unique_section_names);
+  Opts.UniqueBBSectionNames = Args.hasArg(OPT_funique_bb_section_names);
 
   Opts.MergeFunctions = Args.hasArg(OPT_fmerge_functions);
 
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -4202,8 +4202,11 @@
 options::OPT_fno_function_sections,
 options::OPT_fdata_sections,
 options::OPT_fno_data_sections,
+options::OPT_fbasicblock_sections_EQ,
 options::OPT_funique_section_names,
 options::OPT_fno_unique_section_names,
+options::OPT_funique_bb_section_names,
+options::OPT_fno_unique_bb_section_names,
 options::OPT_mrestrict_it,
 options::OPT_mno_restrict_it,
 options::OPT_mstackrealign,
@@ -4758,6 +4761,12 @@
 CmdArgs.push_back("-ffunction-sections");
   }
 
+
+  if (Arg *A = Args.getLastArg(options::OPT_fbasicblock_sections_EQ)) {
+CmdArgs.push_back(
+Args.MakeArgString(Twine("-fbasicblock-sections=") + A->getValue()));
+  }
+
   if (Args.hasFlag(options::OPT_fdata_sections, options::OPT_fno_data_sections,
UseSeparateSections)) {
 CmdArgs.push_back("-fdata-sections");
@@ -4767,6 

[PATCH] D73629: [analyzer] vfork checker: allow execve after vfork

[Jan Včelák via Phabricator via cfe-commits]

janvcelak added a comment.
Herald added a subscriber: martong.

Please, can the reviewers respond to my comments? I would like to know if this 
needs additional revision or if it can be accepted as is.


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

https://reviews.llvm.org/D73629



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

[PATCH] D74433: [llvm-objdump] Print file format in lowercase to match GNU output.

[Jordan Rupprecht via Phabricator via cfe-commits]

rupprecht created this revision.
rupprecht added reviewers: MaskRay, jhenderson.
Herald added subscribers: llvm-commits, cfe-commits, kerbowa, nhaehnle, jvesely.
Herald added a reviewer: alexshap.
Herald added projects: clang, LLVM.

GNU objdump prints the file format in lowercase, e.g. `elf64-x86-64`. 
llvm-objdump prints `ELF64-x86-64` right now, even though piping that into 
llvm-objcopy refuses that as a valid arch to use.

As an example of a problem this causes, see: 
https://github.com/ClangBuiltLinux/linux/issues/779


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D74433

Files:
  clang/test/Modules/pch_container.m
  lld/test/COFF/savetemps.ll
  llvm/test/CodeGen/AArch64/arm64-simplest-elf.ll
  llvm/test/CodeGen/ARM/Windows/trivial-gnu-object.ll
  llvm/test/CodeGen/BPF/reloc-btf-2.ll
  llvm/test/CodeGen/BPF/reloc-btf.ll
  llvm/test/CodeGen/BPF/reloc.ll
  llvm/test/Object/AMDGPU/objdump.s
  llvm/test/Object/X86/objdump-disassembly-inline-relocations.test
  llvm/test/Object/X86/objdump-label.test
  llvm/test/Object/X86/objdump-trivial-object.test
  llvm/test/Object/dynamic-reloc.test
  llvm/test/Object/objdump-symbol-table.test
  llvm/test/tools/llvm-objdump/X86/disassemble-section-name.s
  llvm/test/tools/llvm-objdump/X86/elf-disassemble-symbol-labels-exec.test
  llvm/test/tools/llvm-objdump/X86/elf-dynamic-relocs.test
  llvm/test/tools/llvm-objdump/X86/output-ordering.test
  llvm/test/tools/llvm-objdump/X86/warn-missing-disasm-func.test
  llvm/test/tools/llvm-objdump/all-headers.test
  llvm/test/tools/llvm-objdump/archive-headers.test
  llvm/test/tools/llvm-objdump/file-headers-coff.test
  llvm/test/tools/llvm-objdump/file-headers-elf.test
  llvm/test/tools/llvm-objdump/non-archive-object.test
  llvm/test/tools/llvm-objdump/relocations-in-nonreloc.test
  llvm/tools/llvm-objdump/llvm-objdump.cpp

Index: llvm/tools/llvm-objdump/llvm-objdump.cpp
===
--- llvm/tools/llvm-objdump/llvm-objdump.cpp
+++ llvm/tools/llvm-objdump/llvm-objdump.cpp
@@ -2172,7 +2172,9 @@
   outs() << A->getFileName() << "(" << O->getFileName() << ")";
 else
   outs() << O->getFileName();
-outs() << ":\tfile format " << O->getFileFormatName() << "\n\n";
+outs() << ":\tfile format ";
+printLowerCase(O->getFileFormatName(), outs());
+outs() << "\n\n";
   }
 
   if (StartAddress.getNumOccurrences() || StopAddress.getNumOccurrences())
Index: llvm/test/tools/llvm-objdump/relocations-in-nonreloc.test
===
--- llvm/test/tools/llvm-objdump/relocations-in-nonreloc.test
+++ llvm/test/tools/llvm-objdump/relocations-in-nonreloc.test
@@ -7,7 +7,7 @@
 # RUN: yaml2obj --docnum=3 %s -o %t3
 # RUN: llvm-objdump -r %t3 | FileCheck %s -DFILE=%t3 --check-prefixes=FMT,REL --implicit-check-not={{.}}
 
-# FMT: [[FILE]]: file format ELF64-x86-64
+# FMT: [[FILE]]: file format elf64-x86-64
 
 # REL:  RELOCATION RECORDS FOR []:
 # REL-NEXT: 0123 R_X86_64_NONE *ABS*+0x141
Index: llvm/test/tools/llvm-objdump/non-archive-object.test
===
--- llvm/test/tools/llvm-objdump/non-archive-object.test
+++ llvm/test/tools/llvm-objdump/non-archive-object.test
@@ -2,7 +2,7 @@
 # RUN: llvm-objdump -a %t | FileCheck %s
 
 # If this test has not crashed, then this test passed.
-# CHECK: file format ELF64-x86-64
+# CHECK: file format elf64-x86-64
 
 !ELF
 FileHeader:
Index: llvm/test/tools/llvm-objdump/file-headers-elf.test
===
--- llvm/test/tools/llvm-objdump/file-headers-elf.test
+++ llvm/test/tools/llvm-objdump/file-headers-elf.test
@@ -10,7 +10,7 @@
   Machine: EM_X86_64
   Entry:   0x123456789abcde
 
-# ELF64: [[FILE]]: file format ELF64-x86-64
+# ELF64: [[FILE]]: file format elf64-x86-64
 # ELF64: architecture: x86_64
 # ELF64: start address: 0x00123456789abcde
 
@@ -18,7 +18,7 @@
 # RUN: llvm-objdump -f %t-i386 | FileCheck -DFILE=%t-i386 %s -check-prefix ELF32
 # RUN: llvm-objdump -file-headers %t-i386 | FileCheck %s -DFILE=%t-i386 -check-prefix ELF32
 
-# ELF32: [[FILE]]: file format ELF32-i386
+# ELF32: [[FILE]]: file format elf32-i386
 # ELF32: architecture: i386
 # ELF32: start address: 0x12345678
 
Index: llvm/test/tools/llvm-objdump/file-headers-coff.test
===
--- llvm/test/tools/llvm-objdump/file-headers-coff.test
+++ llvm/test/tools/llvm-objdump/file-headers-coff.test
@@ -9,6 +9,6 @@
 sections:
 symbols:
 
-# CHECK: [[FILE]]: file format COFF-i386
+# CHECK: [[FILE]]: file format coff-i386
 # CHECK: architecture: i386
 # CHECK: start address: 0x
Index: llvm/test/tools/llvm-objdump/archive-headers.test
===
--- llvm/test/tools/llvm-objdump/archive-headers.test
+++ 

[PATCH] D74374: [clang-tidy] Added check to disable bugprone-infinite-loop on known false condition

[Nathan James via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGc69ec6476806: [clang-tidy] Added check to disable 
bugprone-infinite-loop on known false… (authored by njames93).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74374

Files:
  clang-tools-extra/clang-tidy/bugprone/InfiniteLoopCheck.cpp
  clang-tools-extra/test/clang-tidy/checkers/bugprone-infinite-loop.cpp


Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-infinite-loop.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/bugprone-infinite-loop.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-infinite-loop.cpp
@@ -354,3 +354,12 @@
 (*p)++;
   } while (i < Limit);
 }
+
+void evaluatable(bool CondVar) {
+  for (; false && CondVar;) {
+  }
+  while (false && CondVar) {
+  }
+  do {
+  } while (false && CondVar);
+}
Index: clang-tools-extra/clang-tidy/bugprone/InfiniteLoopCheck.cpp
===
--- clang-tools-extra/clang-tidy/bugprone/InfiniteLoopCheck.cpp
+++ clang-tools-extra/clang-tidy/bugprone/InfiniteLoopCheck.cpp
@@ -152,6 +152,13 @@
   return Result;
 }
 
+static bool isKnownFalse(const Expr , const ASTContext ) {
+  bool Result = false;
+  if (Cond.EvaluateAsBooleanCondition(Result, Ctx))
+return !Result;
+  return false;
+}
+
 void InfiniteLoopCheck::registerMatchers(MatchFinder *Finder) {
   const auto LoopCondition = allOf(
   hasCondition(
@@ -170,6 +177,9 @@
   const auto *LoopStmt = Result.Nodes.getNodeAs("loop-stmt");
   const auto *Func = Result.Nodes.getNodeAs("func");
 
+  if (isKnownFalse(*Cond, *Result.Context))
+return;
+
   bool ShouldHaveConditionVariables = true;
   if (const auto *While = dyn_cast(LoopStmt)) {
 if (const VarDecl *LoopVarDecl = While->getConditionVariable()) {


Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-infinite-loop.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/bugprone-infinite-loop.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-infinite-loop.cpp
@@ -354,3 +354,12 @@
 (*p)++;
   } while (i < Limit);
 }
+
+void evaluatable(bool CondVar) {
+  for (; false && CondVar;) {
+  }
+  while (false && CondVar) {
+  }
+  do {
+  } while (false && CondVar);
+}
Index: clang-tools-extra/clang-tidy/bugprone/InfiniteLoopCheck.cpp
===
--- clang-tools-extra/clang-tidy/bugprone/InfiniteLoopCheck.cpp
+++ clang-tools-extra/clang-tidy/bugprone/InfiniteLoopCheck.cpp
@@ -152,6 +152,13 @@
   return Result;
 }
 
+static bool isKnownFalse(const Expr , const ASTContext ) {
+  bool Result = false;
+  if (Cond.EvaluateAsBooleanCondition(Result, Ctx))
+return !Result;
+  return false;
+}
+
 void InfiniteLoopCheck::registerMatchers(MatchFinder *Finder) {
   const auto LoopCondition = allOf(
   hasCondition(
@@ -170,6 +177,9 @@
   const auto *LoopStmt = Result.Nodes.getNodeAs("loop-stmt");
   const auto *Func = Result.Nodes.getNodeAs("func");
 
+  if (isKnownFalse(*Cond, *Result.Context))
+return;
+
   bool ShouldHaveConditionVariables = true;
   if (const auto *While = dyn_cast(LoopStmt)) {
 if (const VarDecl *LoopVarDecl = While->getConditionVariable()) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] c69ec64 - [clang-tidy] Added check to disable bugprone-infinite-loop on known false condition

[Nathan James via cfe-commits]

Author: Nathan James
Date: 2020-02-11T19:37:11Z
New Revision: c69ec6476806147e46bf09b693acb24177982dc2

URL: 
https://github.com/llvm/llvm-project/commit/c69ec6476806147e46bf09b693acb24177982dc2
DIFF: 
https://github.com/llvm/llvm-project/commit/c69ec6476806147e46bf09b693acb24177982dc2.diff

LOG: [clang-tidy] Added check to disable bugprone-infinite-loop on known false 
condition

Summary: Addresses [[ https://bugs.llvm.org/show_bug.cgi?id=44816 | 
bugprone-infinite-loop false positive with CATCH2 ]] by disabling the check on 
loops where the condition is known to always eval as false, in other words not 
a loop.

Reviewers: aaron.ballman, alexfh, hokein, gribozavr2, JonasToth

Reviewed By: gribozavr2

Subscribers: xazax.hun, cfe-commits

Tags: #clang, #clang-tools-extra

Differential Revision: https://reviews.llvm.org/D74374

Added: 


Modified: 
clang-tools-extra/clang-tidy/bugprone/InfiniteLoopCheck.cpp
clang-tools-extra/test/clang-tidy/checkers/bugprone-infinite-loop.cpp

Removed: 




diff  --git a/clang-tools-extra/clang-tidy/bugprone/InfiniteLoopCheck.cpp 
b/clang-tools-extra/clang-tidy/bugprone/InfiniteLoopCheck.cpp
index 5a705376cd4b..00793009ed39 100644
--- a/clang-tools-extra/clang-tidy/bugprone/InfiniteLoopCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/InfiniteLoopCheck.cpp
@@ -152,6 +152,13 @@ static std::string getCondVarNames(const Stmt *Cond) {
   return Result;
 }
 
+static bool isKnownFalse(const Expr , const ASTContext ) {
+  bool Result = false;
+  if (Cond.EvaluateAsBooleanCondition(Result, Ctx))
+return !Result;
+  return false;
+}
+
 void InfiniteLoopCheck::registerMatchers(MatchFinder *Finder) {
   const auto LoopCondition = allOf(
   hasCondition(
@@ -170,6 +177,9 @@ void InfiniteLoopCheck::check(const 
MatchFinder::MatchResult ) {
   const auto *LoopStmt = Result.Nodes.getNodeAs("loop-stmt");
   const auto *Func = Result.Nodes.getNodeAs("func");
 
+  if (isKnownFalse(*Cond, *Result.Context))
+return;
+
   bool ShouldHaveConditionVariables = true;
   if (const auto *While = dyn_cast(LoopStmt)) {
 if (const VarDecl *LoopVarDecl = While->getConditionVariable()) {

diff  --git 
a/clang-tools-extra/test/clang-tidy/checkers/bugprone-infinite-loop.cpp 
b/clang-tools-extra/test/clang-tidy/checkers/bugprone-infinite-loop.cpp
index d89bdadf0212..427b5f0272b9 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/bugprone-infinite-loop.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone-infinite-loop.cpp
@@ -354,3 +354,12 @@ void lambda_capture() {
 (*p)++;
   } while (i < Limit);
 }
+
+void evaluatable(bool CondVar) {
+  for (; false && CondVar;) {
+  }
+  while (false && CondVar) {
+  }
+  do {
+  } while (false && CondVar);
+}



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

[PATCH] D71241: [OpenMP][WIP] Use overload centric declare variants

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added a comment.

In D71241#1870218 , @rsmith wrote:

> In D71241#1788003 , @jdoerfert wrote:
>
> > In D71241#1787652 , @hfinkel wrote:
> >
> > > In D71241#1787571 , @ABataev 
> > > wrote:
> > >
> > > > In D71241#1787265 , @hfinkel 
> > > > wrote:
> > > >
> > > > > In D71241#1786959 , 
> > > > > @jdoerfert wrote:
> > > > >
> > > > > > In D71241#1786530 , 
> > > > > > @ABataev wrote:
> > > > > >
> > > > > > > Most probably, we can use this solution without adding a new 
> > > > > > > expression. `DeclRefExpr` class can contain 2 decls: FoundDecl 
> > > > > > > and the Decl being used. You can use FoundDecl to point to the 
> > > > > > > original function and used decl to point to the function being 
> > > > > > > called in this context. But at first, we need to be sure that we 
> > > > > > > can handle all corner cases correctly.
> > > > > >
> > > > > >
> > > > > > What new expression are you talking about?
> > > > >
> > > > >
> > > > > To be clear, I believe he's talking about the new expression that I 
> > > > > proposed we add in order to represent this kind of call. If that's 
> > > > > not needed, and we can use the FoundDecl/Decl pair for that purpose, 
> > > > > that should also be considered.
> > > > >
> > > > > > This solution already does point to both declarations, as shown 
> > > > > > here: https://reviews.llvm.org/D71241#1782504
> > > >
> > > >
> > > > Exactly. We need to check if the `MemberRefExpr` can do this too to 
> > > > handle member functions correctly.
> > > >  And need to wait for opinion from Richard Smith about the design. We 
> > > > need to be sure that it won't break compatibility with C/C++ in some 
> > > > corner cases. Design bugs are very hard to solve and I want to be sure 
> > > > that we don't miss anything. And we provide full compatibility with 
> > > > both C and C++.
> > >
> >
>
>
> I've read through some of the relevant parts of the OpenMP 5.0 specification 
> (but not the 5.1 specification), and it looks like this is the same kind of 
> language-specific function resolution that we do in C++: name lookup finds 
> one declaration, which we then statically resolve to a different declaration. 
> As with the C++ case, it seems reasonable and useful to me to represent the 
> statically-selected callee in the AST as the chosen declaration in the 
> `DeclRefExpr` -- that will be the most useful thing for tooling, static 
> analysis, and so on.
>
> However, that seems to lose information in some cases. Consider this:
>
>   void f(int) {}
>  
>   template void g(T) {}
>  
>   #pragma omp declare variant(f) match(implementation = {vendor(llvm)})
>   template<> void g(int) {}
>  
>   void h() { g(0); }
>
>
> Here, `h()` calls `f(int)`. The approach in this patch will form a 
> `DeclRefExpr` whose `FoundDecl` is the `FunctionTemplateDecl` `g`, and 
> whose resolved declaration is `f(int)`, but that has no reference to `g` 
> (where the `declare variant` appears). That seems like it could be annoying 
> for some tooling uses to deal with; there's no real way to get back to 
> `g` without redoing template argument deduction or similar.
>
> One possibility to improve the representation would be to replace the 
> existing `NamedDecl*` storage for `FoundDecl`s with a 
> `PointerUnion`, where a 
> `OpenMPFoundVariantDecl` is an `ASTContext`-allocated struct listing the 
> original found declaration and the function with the `declare variant` pragma.


Hi Richard, thanks for your answer. I agree that this is the best option.

> 
> 
>>> We do need to be careful here. For cases with FoundDecl != Decl, I think 
>>> that the typo-correction cases look like they probably work, but there are 
>>> a few places where we make semantic decisions based on the mismatch, such 
>>> as:
>>> 
>>> In SemaTemplate.cpp below line 512, we have (this is in C++03-specific 
>>> code):
>>> 
>>>   } else if (!Found.isSuppressingDiagnostics()) {
>>> //   - if the name found is a class template, it must refer to the same
>>> // entity as the one found in the class of the object expression,
>>> // otherwise the program is ill-formed.
>>> if (!Found.isSingleResult() ||
>>> getAsTemplateNameDecl(Found.getFoundDecl())->getCanonicalDecl() !=
>>> OuterTemplate->getCanonicalDecl()) {
>>>   Diag(Found.getNameLoc(),
>>>diag::ext_nested_name_member_ref_lookup_ambiguous)
> 
> This case is only concerned with type templates, so we don't need to worry 
> about it.
> 
>>> and in SemaExpr.cpp near line 2783, we have:
>>> 
>>>   // If we actually found the member through a using declaration, cast
>>>   // down to the using declaration's type.
>>>   //
>>>   // Pointer equality 

[PATCH] D74423: Use C++14-style return type deduction in clang.

[Benjamin Kramer via Phabricator via cfe-commits]

bkramer accepted this revision.
bkramer added a comment.
This revision is now accepted and ready to land.

lg


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74423



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

[PATCH] D72872: [ObjC generics] Fix not inheriting type bounds in categories/extensions.

[Volodymyr Sapsai via Phabricator via cfe-commits]

vsapsai added a comment.

In D72872#1868989 , @hans wrote:

> I don't have the context here. Was I added as a subscriber because it's 
> related to the clang 10 release?


It's not related to clang 10 release. I've added you because earlier you've 
found a problem with a previous approach 
https://github.com/llvm/llvm-project/commit/4c539e8da1b3de38a53ef3f7497f5c45a3243b61
 So in case it breaks something else, you have extra visibility into the change.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72872



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

[PATCH] D71241: [OpenMP][WIP] Use overload centric declare variants

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added a comment.

In D71241#1788003 , @jdoerfert wrote:

> In D71241#1787652 , @hfinkel wrote:
>
> > In D71241#1787571 , @ABataev wrote:
> >
> > > In D71241#1787265 , @hfinkel 
> > > wrote:
> > >
> > > > In D71241#1786959 , @jdoerfert 
> > > > wrote:
> > > >
> > > > > In D71241#1786530 , @ABataev 
> > > > > wrote:
> > > > >
> > > > > > Most probably, we can use this solution without adding a new 
> > > > > > expression. `DeclRefExpr` class can contain 2 decls: FoundDecl and 
> > > > > > the Decl being used. You can use FoundDecl to point to the original 
> > > > > > function and used decl to point to the function being called in 
> > > > > > this context. But at first, we need to be sure that we can handle 
> > > > > > all corner cases correctly.
> > > > >
> > > > >
> > > > > What new expression are you talking about?
> > > >
> > > >
> > > > To be clear, I believe he's talking about the new expression that I 
> > > > proposed we add in order to represent this kind of call. If that's not 
> > > > needed, and we can use the FoundDecl/Decl pair for that purpose, that 
> > > > should also be considered.
> > > >
> > > > > This solution already does point to both declarations, as shown here: 
> > > > > https://reviews.llvm.org/D71241#1782504
> > >
> > >
> > > Exactly. We need to check if the `MemberRefExpr` can do this too to 
> > > handle member functions correctly.
> > >  And need to wait for opinion from Richard Smith about the design. We 
> > > need to be sure that it won't break compatibility with C/C++ in some 
> > > corner cases. Design bugs are very hard to solve and I want to be sure 
> > > that we don't miss anything. And we provide full compatibility with both 
> > > C and C++.
> >
>


I've read through some of the relevant parts of the OpenMP 5.0 specification 
(but not the 5.1 specification), and it looks like this is the same kind of 
language-specific function resolution that we do in C++: name lookup finds one 
declaration, which we then statically resolve to a different declaration. As 
with the C++ case, it seems reasonable and useful to me to represent the 
statically-selected callee in the AST as the chosen declaration in the 
`DeclRefExpr` -- that will be the most useful thing for tooling, static 
analysis, and so on.

However, that seems to lose information in some cases. Consider this:

  void f(int) {}
  
  template void g(T) {}
  
  #pragma omp declare variant(f) match(implementation = {vendor(llvm)})
  template<> void g(int) {}
  
  void h() { g(0); }

Here, `h()` calls `f(int)`. The approach in this patch will form a 
`DeclRefExpr` whose `FoundDecl` is the `FunctionTemplateDecl` `g`, and whose 
resolved declaration is `f(int)`, but that has no reference to `g` (where 
the `declare variant` appears). That seems like it could be annoying for some 
tooling uses to deal with; there's no real way to get back to `g` without 
redoing template argument deduction or similar.

One possibility to improve the representation would be to replace the existing 
`NamedDecl*` storage for `FoundDecl`s with a `PointerUnion`, where a `OpenMPFoundVariantDecl` is an 
`ASTContext`-allocated struct listing the original found declaration and the 
function with the `declare variant` pragma.

>> We do need to be careful here. For cases with FoundDecl != Decl, I think 
>> that the typo-correction cases look like they probably work, but there are a 
>> few places where we make semantic decisions based on the mismatch, such as:
>> 
>> In SemaTemplate.cpp below line 512, we have (this is in C++03-specific code):
>> 
>>   } else if (!Found.isSuppressingDiagnostics()) {
>> //   - if the name found is a class template, it must refer to the same
>> // entity as the one found in the class of the object expression,
>> // otherwise the program is ill-formed.
>> if (!Found.isSingleResult() ||
>> getAsTemplateNameDecl(Found.getFoundDecl())->getCanonicalDecl() !=
>> OuterTemplate->getCanonicalDecl()) {
>>   Diag(Found.getNameLoc(),
>>diag::ext_nested_name_member_ref_lookup_ambiguous)

This case is only concerned with type templates, so we don't need to worry 
about it.

>> and in SemaExpr.cpp near line 2783, we have:
>> 
>>   // If we actually found the member through a using declaration, cast
>>   // down to the using declaration's type.
>>   //
>>   // Pointer equality is fine here because only one declaration of a
>>   // class ever has member declarations.
>>   if (FoundDecl->getDeclContext() != Member->getDeclContext()) {
>> assert(isa(FoundDecl));
>> QualType URecordType = Context.getTypeDeclType(
>>cast(FoundDecl->getDeclContext()));
> 
> Could you specify what behavior you 

[PATCH] D72857: [SYCL] Driver option to enable SYCL mode and select SYCL version

[Alexey Bader via Phabricator via cfe-commits]

bader marked 2 inline comments as done.
bader added inline comments.



Comment at: clang/lib/Frontend/CompilerInvocation.cpp:2548
+  Opts.SYCL = Args.hasFlag(options::OPT_fsycl, options::OPT_fno_sycl, false);
+  Opts.SYCLIsDevice = Args.hasArg(options::OPT_fsycl_is_device);
+  if (Opts.SYCL || Opts.SYCLIsDevice) {

ABataev wrote:
> bader wrote:
> > ABataev wrote:
> > > bader wrote:
> > > > ABataev wrote:
> > > > > bader wrote:
> > > > > > ABataev wrote:
> > > > > > > bader wrote:
> > > > > > > > ABataev wrote:
> > > > > > > > > This option also must be controlled by `-fsycl`:
> > > > > > > > > ```
> > > > > > > > > Opts.SYCLIsDevice =  Opts.SYCL && 
> > > > > > > > > Args.hasArg(options::OPT_fsycl_is_device);
> > > > > > > > > 
> > > > > > > > > ```
> > > > > > > > Does it really has to? This logic is already present in the 
> > > > > > > > driver and it makes front-end tests verbose `%clang_cc1 -fsycl 
> > > > > > > > -fsycl-is-device`.
> > > > > > > > Can `-fsycl-is-device` imply `-fsycl`?
> > > > > > > > Looking how CUDA/OpenMP options are handled, not all of them 
> > > > > > > > are processed using this pattern.
> > > > > > > In general, this is how we handle it in OpenMP. Cuda works 
> > > > > > > differently, because it has its own kind of files (.cu) and Cuda 
> > > > > > > is triggered by the language switch (-x cu). Seems to me, you're 
> > > > > > > using something close to OpenMP model, no? Or do you want to 
> > > > > > > define your own language kind just like Cuda?
> > > > > > I applied you suggest, although I don't fully understand the need 
> > > > > > of using two options instead of two. I would prefer having 
> > > > > > following code:
> > > > > > ```
> > > > > > Opts.SYCLIsDevice = Args.hasArg(options::OPT_fsycl_is_device);
> > > > > > Opts.SYCL = Args.hasArg(options::OPT_fsycl) || Opts.SYCLIsDevice; 
> > > > > > // -fsycl-is-device enable SYCL mode as well
> > > > > > ```
> > > > > I'm not quite familiar with SYCL model, maybe this the right 
> > > > > approach. You'd better try to provide more details. Are there any 
> > > > > differences between just SYCL and SYCL-device compilation modes? How 
> > > > > do you see the compilation sequence in general? At first you're going 
> > > > > to compile the host version of the code, then the device? OR 
> > > > > something different?
> > > > I think SYCL model is quite similar to OpenMP model. One significant 
> > > > difference might be that to implement standard SYCL functionality we 
> > > > don't need any modifications for the host compiler. AFAIK, OpenMP 
> > > > compiler has to support OpenMP pragmas. 
> > > > We have a few attributes for Intel FPGA devices, which we can't 
> > > > pre-process with `__SYCL_DEVICE_ONLY__` macro and we have added 
> > > > "SYCL-host" mode to suppress compiler warnings about attributes ignored 
> > > > on the host. I think there might be other options how this can be 
> > > > achieved w/o adding new compilation mode and use regular C++ front-end 
> > > > as SYCL host compiler.
> > > > I think there is a difference between SYCL and SYCL-device modes, but 
> > > > it's mostly changes the compilation workflow in the driver, but not in 
> > > > the front-end. In SYCL-device mode, driver invokes only one front-end 
> > > > instance to generate offload code. In SYCL mode, driver invokes 
> > > > multiple front-end instances: one in SYCL-device mode and one in 
> > > > regular C++ mode (to be accurate we use SYCL-host mode, but as I 
> > > > mentioned above I don't think it really needed).
> > > > I hope it makes it clear now. Let me know if you have any other 
> > > > questions.
> > > > 
> > > > Do I understand it correctly that OpenMP option enables OpenMP mode, 
> > > > which is equivalent of "SYCL-host" mode and enabling both OpenMP and 
> > > > OpenMPIsDevice is required for enabling OpenMP mode, which is similar 
> > > > to SYCL-device mode?
> > > > If so, can we assume that OpenMPIsDevice implies that OpenMP option is 
> > > > also set (implicitly)?
> > > > Do I understand it correctly that OpenMP option enables OpenMP mode, 
> > > > which is equivalent of "SYCL-host" mode and enabling both OpenMP and 
> > > > OpenMPIsDevice is required for enabling OpenMP mode, which is similar 
> > > > to SYCL-device mode?
> > > 
> > > Well, for driver you need to pass `-fopenmp 
> > > -fopenmp-target=` to enable the compilation with 
> > > offloading support. In the frontend the host part is compiled with 
> > > `-fopenmp` only (+ aux-triple, probably), for devices - `-fopenmp 
> > > -fopenmp-is-device`. Without `-fopenmp` `-fopenmp-is-device` is just 
> > > ignored.
> > What is the reason to require the driver to pass both options for the 
> > devices? It sounds like `-fopenmp-is-device` should be enough to 
> > differentiate from the host part (`-fopenmp` only). Right?
> We treat a little bit differently. `-fopenmp` turns support for OpenMP, while 
> `-fopenmp-is-device` turns 

[PATCH] D74015: [AIX][Frontend] C++ ABI customizations for AIX boilerplate

[Steven Wan via Phabricator via cfe-commits]

stevewan added inline comments.



Comment at: clang/lib/CodeGen/ItaniumCXXABI.cpp:520
+
+class XLClangCXXABI final : public ItaniumCXXABI {
+public:

The class name here is inconsistent with how our ABI kind was called 
previously, as David pointed out. Maybe rename the ABI kind `XLClang`? 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74015



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

[PATCH] D71920: [AST] Refactor propagation of dependency bits. NFC

[Sam McCall via Phabricator via cfe-commits]

sammccall commandeered this revision.
sammccall added a reviewer: ilya-biryukov.
sammccall added a comment.

@ilya-biryukov isn't working on clang fulltime anymore :-(

I'm going to try to pick up the work on RecoveryExpr, starting with this patch.

@rsmith I think this is waiting on your review (sorry if I've missed anything 
else that was outstanding).

If you'd like to have `concept::Requirement` use a similar bitfield, I'd just 
like to confirm my understanding of the current code before refactoring it:

- there's just one `Dependent` bit (along with `UnexpandedPack`) - 
instantiation-dependence isn't relevant?
- RequiresExpr is only instantiation-dependent if value-dependent (unlike other 
exprs)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71920



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

[PATCH] D73242: [WPD/LowerTypeTests] Delay lowering/removal of type tests until after ICP

[Teresa Johnson via Phabricator via cfe-commits]

tejohnson marked an inline comment as done.
tejohnson added a comment.

In D73242#1861125 , @tejohnson wrote:

> In D73242#186 , @thakis wrote:
>
> > This makes lld crash when linking chromium's base_unittests and probably 
> > does the same for most other binaries that use both thinlto and cfi: 
> > https://bugs.chromium.org/p/chromium/issues/detail?id=1049434
>
>
> Reverted at 25aa2eef993e17708889abf56ed1ffad5074a9f4 
> . Will 
> investigate using repro @thakis sent off patch.


Recommitted with fix and additional test case at 
80d0a137a5aba6998fadb764f1e11cb901aae233 
.




Comment at: llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp:1999
+WholeProgramDevirtResolution *Res = nullptr;
+if (ExportSummary && isa(S.first.TypeID))
+  // Create the type id summary resolution regardlness of whether we can

The fix needed for the Chromium issue was to guard the TypeIdSummary creation 
here by whether the TypeID exists in the TypeIdMap (which makes it match the 
comment in fact), as we don't want to create a summary if the type id is not 
used on a global (in which case it should in fact be Unsat). The equivalent 
code in the index-only WPD is essentially already guarded by that condition, 
because of the way the CallSlots are created (and in fact there is an assert in 
that code that we have a use on a vtable, i.e. that a 
TypeIdCompatibleVtableSummary is found for the TypeID).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73242



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

[PATCH] D74412: Fix SFINAE in CFG.cpp.

[Justin Lebar via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGf0fd852fcd05: Fix SFINAE in CFG.cpp. (authored by jlebar).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74412

Files:
  clang/lib/Analysis/CFG.cpp


Index: clang/lib/Analysis/CFG.cpp
===
--- clang/lib/Analysis/CFG.cpp
+++ clang/lib/Analysis/CFG.cpp
@@ -720,10 +720,10 @@
   // These sorts of call expressions don't have a common superclass,
   // hence strict duck-typing.
   template ::value ||
-std::is_same::value ||
-std::is_same::value>>
+typename = std::enable_if_t<
+std::is_base_of::value ||
+std::is_base_of::value ||
+std::is_base_of::value>>
   void findConstructionContextsForArguments(CallLikeExpr *E) {
 for (unsigned i = 0, e = E->getNumArgs(); i != e; ++i) {
   Expr *Arg = E->getArg(i);


Index: clang/lib/Analysis/CFG.cpp
===
--- clang/lib/Analysis/CFG.cpp
+++ clang/lib/Analysis/CFG.cpp
@@ -720,10 +720,10 @@
   // These sorts of call expressions don't have a common superclass,
   // hence strict duck-typing.
   template ::value ||
-std::is_same::value ||
-std::is_same::value>>
+typename = std::enable_if_t<
+std::is_base_of::value ||
+std::is_base_of::value ||
+std::is_base_of::value>>
   void findConstructionContextsForArguments(CallLikeExpr *E) {
 for (unsigned i = 0, e = E->getNumArgs(); i != e; ++i) {
   Expr *Arg = E->getArg(i);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D71920: [AST] Refactor propagation of dependency bits. NFC

[Sam McCall via Phabricator via cfe-commits]

sammccall updated this revision to Diff 243915.
sammccall added a comment.

Rebase (ConceptSpecializationExpr moved).
Added FIXME to use enum for concept requirement dependence, it's still murky to 
me.
addDependencies -> addDependence, for clarity.
Move variably-modified into TypeDependence bitfield.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71920

Files:
  clang/include/clang/AST/DependencyFlags.h
  clang/include/clang/AST/Expr.h
  clang/include/clang/AST/ExprConcepts.h
  clang/include/clang/AST/NestedNameSpecifier.h
  clang/include/clang/AST/Stmt.h
  clang/include/clang/AST/TemplateBase.h
  clang/include/clang/AST/TemplateName.h
  clang/include/clang/AST/Type.h
  clang/lib/AST/ASTImporter.cpp
  clang/lib/AST/Expr.cpp
  clang/lib/AST/ExprCXX.cpp
  clang/lib/AST/ExprConcepts.cpp
  clang/lib/AST/ExprObjC.cpp
  clang/lib/AST/NestedNameSpecifier.cpp
  clang/lib/AST/TemplateBase.cpp
  clang/lib/AST/TemplateName.cpp
  clang/lib/Sema/SemaOverload.cpp
  clang/lib/Serialization/ASTReaderStmt.cpp

Index: clang/lib/Serialization/ASTReaderStmt.cpp
===
--- clang/lib/Serialization/ASTReaderStmt.cpp
+++ clang/lib/Serialization/ASTReaderStmt.cpp
@@ -11,7 +11,6 @@
 //
 //===--===//
 
-#include "clang/Serialization/ASTRecordReader.h"
 #include "clang/AST/ASTConcept.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/AttrIterator.h"
@@ -22,6 +21,7 @@
 #include "clang/AST/DeclObjC.h"
 #include "clang/AST/DeclTemplate.h"
 #include "clang/AST/DeclarationName.h"
+#include "clang/AST/DependencyFlags.h"
 #include "clang/AST/Expr.h"
 #include "clang/AST/ExprCXX.h"
 #include "clang/AST/ExprObjC.h"
@@ -49,6 +49,7 @@
 #include "clang/Basic/TypeTraits.h"
 #include "clang/Lex/Token.h"
 #include "clang/Serialization/ASTBitCodes.h"
+#include "clang/Serialization/ASTRecordReader.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/SmallVector.h"
@@ -511,10 +512,23 @@
 void ASTStmtReader::VisitExpr(Expr *E) {
   VisitStmt(E);
   E->setType(Record.readType());
-  E->setTypeDependent(Record.readInt());
-  E->setValueDependent(Record.readInt());
-  E->setInstantiationDependent(Record.readInt());
-  E->ExprBits.ContainsUnexpandedParameterPack = Record.readInt();
+
+  // FIXME: write and read all DependentFlags with a single call.
+  bool TypeDependent = Record.readInt();
+  bool ValueDependent = Record.readInt();
+  bool InstantiationDependent = Record.readInt();
+  bool ContainsUnexpandedTemplateParameters = Record.readInt();
+  auto Deps = ExprDependence::None;
+  if (TypeDependent)
+Deps |= ExprDependence::Type;
+  if (ValueDependent)
+Deps |= ExprDependence::Value;
+  if (InstantiationDependent)
+Deps |= ExprDependence::Instantiation;
+  if (ContainsUnexpandedTemplateParameters)
+Deps |= ExprDependence::UnexpandedPack;
+  E->setDependence(Deps);
+
   E->setValueKind(static_cast(Record.readInt()));
   E->setObjectKind(static_cast(Record.readInt()));
   assert(Record.getIdx() == NumExprFields &&
Index: clang/lib/Sema/SemaOverload.cpp
===
--- clang/lib/Sema/SemaOverload.cpp
+++ clang/lib/Sema/SemaOverload.cpp
@@ -10,10 +10,10 @@
 //
 //===--===//
 
-#include "clang/Sema/Overload.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/CXXInheritance.h"
 #include "clang/AST/DeclObjC.h"
+#include "clang/AST/DependencyFlags.h"
 #include "clang/AST/Expr.h"
 #include "clang/AST/ExprCXX.h"
 #include "clang/AST/ExprObjC.h"
@@ -24,6 +24,7 @@
 #include "clang/Basic/TargetInfo.h"
 #include "clang/Sema/Initialization.h"
 #include "clang/Sema/Lookup.h"
+#include "clang/Sema/Overload.h"
 #include "clang/Sema/SemaInternal.h"
 #include "clang/Sema/Template.h"
 #include "clang/Sema/TemplateDeduction.h"
@@ -12718,9 +12719,7 @@
   // base classes.
   CallExpr *CE = CallExpr::Create(Context, Fn, Args, Context.DependentTy,
   VK_RValue, RParenLoc);
-  CE->setTypeDependent(true);
-  CE->setValueDependent(true);
-  CE->setInstantiationDependent(true);
+  CE->addDependence(ExprDependence::TypeValueInstantiation);
   *Result = CE;
   return true;
 }
Index: clang/lib/AST/TemplateName.cpp
===
--- clang/lib/AST/TemplateName.cpp
+++ clang/lib/AST/TemplateName.cpp
@@ -13,6 +13,7 @@
 #include "clang/AST/TemplateName.h"
 #include "clang/AST/DeclBase.h"
 #include "clang/AST/DeclTemplate.h"
+#include "clang/AST/DependencyFlags.h"
 #include "clang/AST/NestedNameSpecifier.h"
 #include "clang/AST/PrettyPrinter.h"
 #include "clang/AST/TemplateBase.h"
@@ -168,52 +169,48 @@
   return TemplateName(Decl);
 }
 
-bool 

[PATCH] D74423: Use C++14-style return type deduction in clang.

[Justin Lebar via Phabricator via cfe-commits]

jlebar created this revision.
jlebar added reviewers: bkramer, MaskRay.
Herald added a reviewer: martong.
Herald added a reviewer: shafik.
Herald added subscribers: cfe-commits, martong.
Herald added a project: clang.

Simplifies the C++11-style "-> decltype(...)" return-type deduction.

Note that you have to be careful about whether the function return type
is `auto` or `decltype(auto)`.  The difference is that bare `auto`
strips const and reference, just like lambda return type deduction.  In
some cases that's what we want (or more likely, we know that the return
type is a value type), but whenever we're wrapping a templated function
which might return a reference, we need to be sure that the return type
is decltype(auto).

No functional change.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D74423

Files:
  clang/lib/AST/ASTImporter.cpp
  clang/lib/StaticAnalyzer/Checkers/GCDAntipatternChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/OSObjectCStyleCast.cpp
  clang/lib/StaticAnalyzer/Checkers/ObjCAutoreleaseWriteChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/PointerSortingChecker.cpp


Index: clang/lib/StaticAnalyzer/Checkers/PointerSortingChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/PointerSortingChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/PointerSortingChecker.cpp
@@ -54,7 +54,7 @@
  OS.str(), Location, Range);
 }
 
-auto callsName(const char *FunctionName) -> decltype(callee(functionDecl())) {
+decltype(auto) callsName(const char *FunctionName) {
   return callee(functionDecl(hasName(FunctionName)));
 }
 
Index: clang/lib/StaticAnalyzer/Checkers/ObjCAutoreleaseWriteChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/ObjCAutoreleaseWriteChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/ObjCAutoreleaseWriteChecker.cpp
@@ -100,8 +100,7 @@
   return std::vector(V.begin(), V.end());
 }
 
-static auto callsNames(std::vector FunctionNames)
--> decltype(callee(functionDecl())) {
+static decltype(auto) callsNames(std::vector FunctionNames) {
   return callee(functionDecl(hasAnyName(toRefs(FunctionNames;
 }
 
Index: clang/lib/StaticAnalyzer/Checkers/OSObjectCStyleCast.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/OSObjectCStyleCast.cpp
+++ clang/lib/StaticAnalyzer/Checkers/OSObjectCStyleCast.cpp
@@ -55,8 +55,7 @@
 CE->getSourceRange());
 }
 
-static auto hasTypePointingTo(DeclarationMatcher DeclM)
--> decltype(hasType(pointerType())) {
+static decltype(auto) hasTypePointingTo(DeclarationMatcher DeclM) {
   return hasType(pointerType(pointee(hasDeclaration(DeclM;
 }
 
Index: clang/lib/StaticAnalyzer/Checkers/GCDAntipatternChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/GCDAntipatternChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/GCDAntipatternChecker.cpp
@@ -52,18 +52,16 @@
 BugReporter ) const;
 };
 
-auto callsName(const char *FunctionName)
--> decltype(callee(functionDecl())) {
+decltype(auto) callsName(const char *FunctionName) {
   return callee(functionDecl(hasName(FunctionName)));
 }
 
-auto equalsBoundArgDecl(int ArgIdx, const char *DeclName)
--> decltype(hasArgument(0, expr())) {
+decltype(auto) equalsBoundArgDecl(int ArgIdx, const char *DeclName) {
   return hasArgument(ArgIdx, ignoringParenCasts(declRefExpr(
  to(varDecl(equalsBoundNode(DeclName));
 }
 
-auto bindAssignmentToDecl(const char *DeclName) -> decltype(hasLHS(expr())) {
+decltype(auto) bindAssignmentToDecl(const char *DeclName) {
   return hasLHS(ignoringParenImpCasts(
  declRefExpr(to(varDecl().bind(DeclName);
 }
Index: clang/lib/AST/ASTImporter.cpp
===
--- clang/lib/AST/ASTImporter.cpp
+++ clang/lib/AST/ASTImporter.cpp
@@ -204,9 +204,7 @@
 
 // Wrapper for an overload set.
 template  struct CallOverloadedCreateFun {
-  template 
-  auto operator()(Args &&... args)
-  -> decltype(ToDeclT::Create(std::forward(args)...)) {
+  template  decltype(auto) operator()(Args &&... args) {
 return ToDeclT::Create(std::forward(args)...);
   }
 };


Index: clang/lib/StaticAnalyzer/Checkers/PointerSortingChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/PointerSortingChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/PointerSortingChecker.cpp
@@ -54,7 +54,7 @@
  OS.str(), Location, Range);
 }
 
-auto callsName(const char *FunctionName) -> decltype(callee(functionDecl())) {
+decltype(auto) callsName(const char *FunctionName) {
   return callee(functionDecl(hasName(FunctionName)));
 }
 
Index: 

[PATCH] D74414: Use std::foo_t rather than std::foo in clang.

[Justin Lebar via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG027eb71696f6: Use std::foo_t rather than std::foo in clang. 
(authored by jlebar).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74414

Files:
  clang/include/clang/AST/ASTTypeTraits.h
  clang/include/clang/AST/CanonicalType.h
  clang/include/clang/AST/DataCollection.h
  clang/include/clang/AST/Expr.h
  clang/include/clang/AST/OpenMPClause.h
  clang/include/clang/AST/RecursiveASTVisitor.h
  clang/include/clang/ASTMatchers/ASTMatchersInternal.h
  clang/include/clang/Analysis/CFG.h
  clang/include/clang/Basic/Diagnostic.h
  clang/include/clang/Basic/PartialDiagnostic.h
  clang/include/clang/Tooling/Refactoring/RefactoringOptions.h
  clang/lib/AST/ASTImporter.cpp
  clang/lib/AST/Interp/Boolean.h
  clang/lib/AST/Interp/Integral.h
  clang/lib/AST/Interp/Source.h
  clang/lib/Analysis/ThreadSafety.cpp
  clang/lib/CodeGen/CGOpenMPRuntime.h
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/lib/Sema/SemaTemplateDeduction.cpp
  clang/lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp
  clang/lib/Tooling/ASTDiff/ASTDiff.cpp
  clang/unittests/Tooling/ASTSelectionTest.cpp

Index: clang/unittests/Tooling/ASTSelectionTest.cpp
===
--- clang/unittests/Tooling/ASTSelectionTest.cpp
+++ clang/unittests/Tooling/ASTSelectionTest.cpp
@@ -101,22 +101,22 @@
 }
 
 template 
-const SelectedASTNode &
-checkNode(const SelectedASTNode , SourceSelectionKind SelectionKind,
-  unsigned NumChildren = 0,
-  typename std::enable_if::value, T>::type
-  *StmtOverloadChecker = nullptr) {
+const SelectedASTNode (
+const SelectedASTNode , SourceSelectionKind SelectionKind,
+unsigned NumChildren = 0,
+std::enable_if_t::value, T> *StmtOverloadChecker =
+nullptr) {
   checkNodeImpl(isa(StmtNode.Node.get()), StmtNode, SelectionKind,
 NumChildren);
   return StmtNode;
 }
 
 template 
-const SelectedASTNode &
-checkNode(const SelectedASTNode , SourceSelectionKind SelectionKind,
-  unsigned NumChildren = 0, StringRef Name = "",
-  typename std::enable_if::value, T>::type
-  *DeclOverloadChecker = nullptr) {
+const SelectedASTNode (
+const SelectedASTNode , SourceSelectionKind SelectionKind,
+unsigned NumChildren = 0, StringRef Name = "",
+std::enable_if_t::value, T> *DeclOverloadChecker =
+nullptr) {
   checkNodeImpl(isa(DeclNode.Node.get()), DeclNode, SelectionKind,
 NumChildren);
   if (!Name.empty())
Index: clang/lib/Tooling/ASTDiff/ASTDiff.cpp
===
--- clang/lib/Tooling/ASTDiff/ASTDiff.cpp
+++ clang/lib/Tooling/ASTDiff/ASTDiff.cpp
@@ -117,12 +117,12 @@
   Impl(SyntaxTree *Parent, Stmt *N, ASTContext );
   template 
   Impl(SyntaxTree *Parent,
-   typename std::enable_if::value, T>::type *Node,
+   std::enable_if_t::value, T> *Node,
ASTContext )
   : Impl(Parent, dyn_cast(Node), AST) {}
   template 
   Impl(SyntaxTree *Parent,
-   typename std::enable_if::value, T>::type *Node,
+   std::enable_if_t::value, T> *Node,
ASTContext )
   : Impl(Parent, dyn_cast(Node), AST) {}
 
Index: clang/lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp
===
--- clang/lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp
+++ clang/lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp
@@ -51,11 +51,10 @@
 } // end of anonymous namespace
 
 template 
-static
-typename std::conditional::value,
-  typename CheckerOrPackageInfoList::const_iterator,
-  typename CheckerOrPackageInfoList::iterator>::type
-binaryFind(CheckerOrPackageInfoList , StringRef FullName) {
+static std::conditional_t::value,
+  typename CheckerOrPackageInfoList::const_iterator,
+  typename CheckerOrPackageInfoList::iterator>
+binaryFind(CheckerOrPackageInfoList , StringRef FullName) {
 
   using CheckerOrPackage = typename CheckerOrPackageInfoList::value_type;
   using CheckerOrPackageFullNameLT = FullNameLT;
Index: clang/lib/Sema/SemaTemplateDeduction.cpp
===
--- clang/lib/Sema/SemaTemplateDeduction.cpp
+++ clang/lib/Sema/SemaTemplateDeduction.cpp
@@ -2747,8 +2747,8 @@
 
 /// Complete template argument deduction for a partial specialization.
 template 
-static typename std::enable_if::value,
-   Sema::TemplateDeductionResult>::type
+static std::enable_if_t::value,
+Sema::TemplateDeductionResult>
 FinishTemplateArgumentDeduction(
 Sema , T *Partial, bool IsPartialOrdering,
 const TemplateArgumentList ,
Index: clang/lib/Sema/SemaDeclAttr.cpp

[clang] 57148e0 - [Hexagon] Fix ABI info for returning HVX vectors

[Krzysztof Parzyszek via cfe-commits]

Author: Krzysztof Parzyszek
Date: 2020-02-11T12:38:54-06:00
New Revision: 57148e0379d30ecabd2a338c5bb9abbb3a0e314f

URL: 
https://github.com/llvm/llvm-project/commit/57148e0379d30ecabd2a338c5bb9abbb3a0e314f
DIFF: 
https://github.com/llvm/llvm-project/commit/57148e0379d30ecabd2a338c5bb9abbb3a0e314f.diff

LOG: [Hexagon] Fix ABI info for returning HVX vectors

Added: 
clang/test/CodeGen/hexagon-hvx-abi.c

Modified: 
clang/lib/CodeGen/TargetInfo.cpp

Removed: 




diff  --git a/clang/lib/CodeGen/TargetInfo.cpp 
b/clang/lib/CodeGen/TargetInfo.cpp
index a3e3a38a1033..29998f361926 100644
--- a/clang/lib/CodeGen/TargetInfo.cpp
+++ b/clang/lib/CodeGen/TargetInfo.cpp
@@ -7486,13 +7486,10 @@ void TCETargetCodeGenInfo::setTargetAttributes(
 namespace {
 
 class HexagonABIInfo : public ABIInfo {
-
-
 public:
   HexagonABIInfo(CodeGenTypes ) : ABIInfo(CGT) {}
 
 private:
-
   ABIArgInfo classifyReturnType(QualType RetTy) const;
   ABIArgInfo classifyArgumentType(QualType RetTy) const;
 
@@ -7505,14 +7502,14 @@ class HexagonABIInfo : public ABIInfo {
 class HexagonTargetCodeGenInfo : public TargetCodeGenInfo {
 public:
   HexagonTargetCodeGenInfo(CodeGenTypes )
-:TargetCodeGenInfo(new HexagonABIInfo(CGT)) {}
+: TargetCodeGenInfo(new HexagonABIInfo(CGT)) {}
 
   int getDwarfEHStackPointer(CodeGen::CodeGenModule ) const override {
 return 29;
   }
 };
 
-}
+} // namespace
 
 void HexagonABIInfo::computeInfo(CGFunctionInfo ) const {
   if (!getCXXABI().classifyReturnType(FI))
@@ -7527,8 +7524,8 @@ ABIArgInfo HexagonABIInfo::classifyArgumentType(QualType 
Ty) const {
 if (const EnumType *EnumTy = Ty->getAs())
   Ty = EnumTy->getDecl()->getIntegerType();
 
-return (Ty->isPromotableIntegerType() ? ABIArgInfo::getExtend(Ty)
-  : ABIArgInfo::getDirect());
+return Ty->isPromotableIntegerType() ? ABIArgInfo::getExtend(Ty)
+ : ABIArgInfo::getDirect();
   }
 
   if (CGCXXABI::RecordArgABI RAA = getRecordArgABI(Ty, getCXXABI()))
@@ -7539,53 +7536,56 @@ ABIArgInfo 
HexagonABIInfo::classifyArgumentType(QualType Ty) const {
 return ABIArgInfo::getIgnore();
 
   uint64_t Size = getContext().getTypeSize(Ty);
-  if (Size > 64)
-return getNaturalAlignIndirect(Ty, /*ByVal=*/true);
+  if (Size <= 64) {
 // Pass in the smallest viable integer type.
-  else if (Size > 32)
-  return ABIArgInfo::getDirect(llvm::Type::getInt64Ty(getVMContext()));
-  else if (Size > 16)
-  return ABIArgInfo::getDirect(llvm::Type::getInt32Ty(getVMContext()));
-  else if (Size > 8)
-  return ABIArgInfo::getDirect(llvm::Type::getInt16Ty(getVMContext()));
-  else
-  return ABIArgInfo::getDirect(llvm::Type::getInt8Ty(getVMContext()));
+if (!llvm::isPowerOf2_64(Size))
+  Size = llvm::NextPowerOf2(Size);
+return ABIArgInfo::getDirect(llvm::Type::getIntNTy(getVMContext(), Size));
+  }
+  return getNaturalAlignIndirect(Ty, /*ByVal=*/true);
 }
 
 ABIArgInfo HexagonABIInfo::classifyReturnType(QualType RetTy) const {
   if (RetTy->isVoidType())
 return ABIArgInfo::getIgnore();
 
-  // Large vector types should be returned via memory.
-  if (RetTy->isVectorType() && getContext().getTypeSize(RetTy) > 64)
-return getNaturalAlignIndirect(RetTy);
+  const TargetInfo  = CGT.getTarget();
+  uint64_t Size = getContext().getTypeSize(RetTy);
+
+  if (const auto *VecTy = RetTy->getAs()) {
+// HVX vectors are returned in vector registers or register pairs.
+if (T.hasFeature("hvx")) {
+  assert(T.hasFeature("hvx-length64b") || T.hasFeature("hvx-length128b"));
+  uint64_t VecSize = T.hasFeature("hvx-length64b") ? 64*8 : 128*8;
+  if (Size == VecSize || Size == 2*VecSize)
+return ABIArgInfo::getDirectInReg();
+}
+
+// Large vector types should be returned via memory.
+if (Size > 64)
+  return getNaturalAlignIndirect(RetTy);
+  }
 
   if (!isAggregateTypeForABI(RetTy)) {
 // Treat an enum type as its underlying type.
 if (const EnumType *EnumTy = RetTy->getAs())
   RetTy = EnumTy->getDecl()->getIntegerType();
 
-return (RetTy->isPromotableIntegerType() ? ABIArgInfo::getExtend(RetTy)
- : ABIArgInfo::getDirect());
+return RetTy->isPromotableIntegerType() ? ABIArgInfo::getExtend(RetTy)
+: ABIArgInfo::getDirect();
   }
 
   if (isEmptyRecord(getContext(), RetTy, true))
 return ABIArgInfo::getIgnore();
 
-  // Aggregates <= 8 bytes are returned in r0; other aggregates
+  // Aggregates <= 8 bytes are returned in registers, other aggregates
   // are returned indirectly.
-  uint64_t Size = getContext().getTypeSize(RetTy);
   if (Size <= 64) {
 // Return in the smallest viable integer type.
-if (Size <= 8)
-  return ABIArgInfo::getDirect(llvm::Type::getInt8Ty(getVMContext()));
-if (Size 

[clang] f0fd852 - Fix SFINAE in CFG.cpp.

[Justin Lebar via cfe-commits]

Author: Justin Lebar
Date: 2020-02-11T10:37:08-08:00
New Revision: f0fd852fcd054297f2b07e2ca87551de9b2a39c0

URL: 
https://github.com/llvm/llvm-project/commit/f0fd852fcd054297f2b07e2ca87551de9b2a39c0
DIFF: 
https://github.com/llvm/llvm-project/commit/f0fd852fcd054297f2b07e2ca87551de9b2a39c0.diff

LOG: Fix SFINAE in CFG.cpp.

Summary: Used std::enable_if without ::type.

Reviewers: bkramer, MaskRay

Subscribers: martong, cfe-commits

Tags: #clang

Differential Revision: https://reviews.llvm.org/D74412

Added: 


Modified: 
clang/lib/Analysis/CFG.cpp

Removed: 




diff  --git a/clang/lib/Analysis/CFG.cpp b/clang/lib/Analysis/CFG.cpp
index 4c1ea8995f9f..8091625703bc 100644
--- a/clang/lib/Analysis/CFG.cpp
+++ b/clang/lib/Analysis/CFG.cpp
@@ -720,10 +720,10 @@ class CFGBuilder {
   // These sorts of call expressions don't have a common superclass,
   // hence strict duck-typing.
   template ::value ||
-std::is_same::value ||
-std::is_same::value>>
+typename = std::enable_if_t<
+std::is_base_of::value ||
+std::is_base_of::value ||
+std::is_base_of::value>>
   void findConstructionContextsForArguments(CallLikeExpr *E) {
 for (unsigned i = 0, e = E->getNumArgs(); i != e; ++i) {
   Expr *Arg = E->getArg(i);



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

[clang] 027eb71 - Use std::foo_t rather than std::foo in clang.

[Justin Lebar via cfe-commits]

Author: Justin Lebar
Date: 2020-02-11T10:37:08-08:00
New Revision: 027eb71696f6ce4fdeb63f68c8c6b66e147ad407

URL: 
https://github.com/llvm/llvm-project/commit/027eb71696f6ce4fdeb63f68c8c6b66e147ad407
DIFF: 
https://github.com/llvm/llvm-project/commit/027eb71696f6ce4fdeb63f68c8c6b66e147ad407.diff

LOG: Use std::foo_t rather than std::foo in clang.

Summary: No functional change.

Reviewers: bkramer, MaskRay, martong, shafik

Subscribers: martong, cfe-commits

Tags: #clang

Differential Revision: https://reviews.llvm.org/D74414

Added: 


Modified: 
clang/include/clang/AST/ASTTypeTraits.h
clang/include/clang/AST/CanonicalType.h
clang/include/clang/AST/DataCollection.h
clang/include/clang/AST/Expr.h
clang/include/clang/AST/OpenMPClause.h
clang/include/clang/AST/RecursiveASTVisitor.h
clang/include/clang/ASTMatchers/ASTMatchersInternal.h
clang/include/clang/Analysis/CFG.h
clang/include/clang/Basic/Diagnostic.h
clang/include/clang/Basic/PartialDiagnostic.h
clang/include/clang/Tooling/Refactoring/RefactoringOptions.h
clang/lib/AST/ASTImporter.cpp
clang/lib/AST/Interp/Boolean.h
clang/lib/AST/Interp/Integral.h
clang/lib/AST/Interp/Source.h
clang/lib/Analysis/ThreadSafety.cpp
clang/lib/CodeGen/CGOpenMPRuntime.h
clang/lib/Sema/SemaDeclAttr.cpp
clang/lib/Sema/SemaTemplateDeduction.cpp
clang/lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp
clang/lib/Tooling/ASTDiff/ASTDiff.cpp
clang/unittests/Tooling/ASTSelectionTest.cpp

Removed: 




diff  --git a/clang/include/clang/AST/ASTTypeTraits.h 
b/clang/include/clang/AST/ASTTypeTraits.h
index 1a12281d039d..777ad2fec349 100644
--- a/clang/include/clang/AST/ASTTypeTraits.h
+++ b/clang/include/clang/AST/ASTTypeTraits.h
@@ -465,22 +465,22 @@ class DynTypedNode {
 
 template 
 struct DynTypedNode::BaseConverter<
-T, typename std::enable_if::value>::type>
+T, std::enable_if_t::value>>
 : public DynCastPtrConverter {};
 
 template 
 struct DynTypedNode::BaseConverter<
-T, typename std::enable_if::value>::type>
+T, std::enable_if_t::value>>
 : public DynCastPtrConverter {};
 
 template 
 struct DynTypedNode::BaseConverter<
-T, typename std::enable_if::value>::type>
+T, std::enable_if_t::value>>
 : public DynCastPtrConverter {};
 
 template 
 struct DynTypedNode::BaseConverter<
-T, typename std::enable_if::value>::type>
+T, std::enable_if_t::value>>
 : public DynCastPtrConverter {};
 
 template <>

diff  --git a/clang/include/clang/AST/CanonicalType.h 
b/clang/include/clang/AST/CanonicalType.h
index 64ec1c0ce471..31b14c0d39c3 100644
--- a/clang/include/clang/AST/CanonicalType.h
+++ b/clang/include/clang/AST/CanonicalType.h
@@ -74,7 +74,7 @@ class CanQual {
   /// canonical type pointers.
   template 
   CanQual(const CanQual ,
-  typename std::enable_if::value, int>::type = 
0);
+  std::enable_if_t::value, int> = 0);
 
   /// Retrieve the underlying type pointer, which refers to a
   /// canonical type.

diff  --git a/clang/include/clang/AST/DataCollection.h 
b/clang/include/clang/AST/DataCollection.h
index 37f101793ecc..14d1bc188623 100644
--- a/clang/include/clang/AST/DataCollection.h
+++ b/clang/include/clang/AST/DataCollection.h
@@ -50,10 +50,9 @@ template  void addDataToConsumer(T , 
const QualType ) {
 }
 
 template 
-typename std::enable_if<
-std::is_integral::value || std::is_enum::value ||
-std::is_convertible::value // for llvm::hash_code
->::type
+std::enable_if_t::value || std::is_enum::value ||
+ std::is_convertible::value // for 
llvm::hash_code
+ >
 addDataToConsumer(T , Type Data) {
   DataConsumer.update(StringRef(reinterpret_cast(), 
sizeof(Data)));
 }

diff  --git a/clang/include/clang/AST/Expr.h b/clang/include/clang/AST/Expr.h
index 5d9174f6e627..fcdb0b992134 100644
--- a/clang/include/clang/AST/Expr.h
+++ b/clang/include/clang/AST/Expr.h
@@ -5282,10 +5282,9 @@ class GenericSelectionExpr final
   template  class AssociationTy {
 friend class GenericSelectionExpr;
 template  friend class AssociationIteratorTy;
-using ExprPtrTy =
-typename std::conditional::type;
-using TSIPtrTy = typename std::conditional::type;
+using ExprPtrTy = std::conditional_t;
+using TSIPtrTy =
+std::conditional_t;
 ExprPtrTy E;
 TSIPtrTy TSI;
 bool Selected;
@@ -5327,10 +5326,9 @@ class GenericSelectionExpr final
 //const Association  = *It++; // Oops, Assoc is dangling.
 using BaseTy = typename AssociationIteratorTy::iterator_facade_base;
 using StmtPtrPtrTy =
-typename std::conditional::type;
-using TSIPtrPtrTy =
-typename std::conditional::type;
+std::conditional_t;
+using TSIPtrPtrTy = std::conditional_t;
 StmtPtrPtrTy E; // = nullptr; FIXME: Once support for gcc 4.8 is dropped.
 TSIPtrPtrTy TSI; // 

[PATCH] D69591: Devirtualize a call on alloca without waiting for post inline cleanup and next DevirtSCCRepeatedPass iteration.

[Hiroshi Yamauchi via Phabricator via cfe-commits]

yamauchi updated this revision to Diff 243911.
yamauchi added a comment.

Rebase past D71308 . PTAL.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69591

Files:
  clang/test/CodeGenCXX/member-function-pointer-calls.cpp
  llvm/lib/Transforms/IPO/Inliner.cpp
  llvm/test/Transforms/Inline/devirtualize-4.ll

Index: llvm/test/Transforms/Inline/devirtualize-4.ll
===
--- /dev/null
+++ llvm/test/Transforms/Inline/devirtualize-4.ll
@@ -0,0 +1,214 @@
+; RUN: opt < %s -passes='cgscc(devirt<4>(inline)),function(sroa,early-cse)' -S | FileCheck %s
+; RUN: opt < %s -passes='default' -S | FileCheck %s
+
+; Check that DoNotOptimize is inlined into Test.
+; CHECK: @_Z4Testv()
+; CHECK-NOT: ret void
+; CHECK: call void asm
+; CHECK: ret void
+
+;template 
+;void DoNotOptimize(const T& var) {
+;  asm volatile("" : "+m"(const_cast(var)));
+;}
+;
+;class Interface {
+; public:
+;  virtual void Run() = 0;
+;};
+;
+;class Impl : public Interface {
+; public:
+;  Impl() : f(3) {}
+;  void Run() { DoNotOptimize(this); }
+;
+; private:
+;  int f;
+;};
+;
+;static void IndirectRun(Interface& o) { o.Run(); }
+;
+;void Test() {
+;  Impl o;
+;  IndirectRun(o);
+;}
+
+%class.Impl = type <{ %class.Interface, i32, [4 x i8] }>
+%class.Interface = type { i32 (...)** }
+
+@_ZTV4Impl = linkonce_odr dso_local unnamed_addr constant { [3 x i8*] } { [3 x i8*] [i8* null, i8* bitcast ({ i8*, i8*, i8* }* @_ZTI4Impl to i8*), i8* bitcast (void (%class.Impl*)* @_ZN4Impl3RunEv to i8*)] }, align 8
+@_ZTVN10__cxxabiv120__si_class_type_infoE = external dso_local global i8*
+@_ZTS4Impl = linkonce_odr dso_local constant [6 x i8] c"4Impl\00", align 1
+@_ZTVN10__cxxabiv117__class_type_infoE = external dso_local global i8*
+@_ZTS9Interface = linkonce_odr dso_local constant [11 x i8] c"9Interface\00", align 1
+@_ZTI9Interface = linkonce_odr dso_local constant { i8*, i8* } { i8* bitcast (i8** getelementptr inbounds (i8*, i8** @_ZTVN10__cxxabiv117__class_type_infoE, i64 2) to i8*), i8* getelementptr inbounds ([11 x i8], [11 x i8]* @_ZTS9Interface, i32 0, i32 0) }, align 8
+@_ZTI4Impl = linkonce_odr dso_local constant { i8*, i8*, i8* } { i8* bitcast (i8** getelementptr inbounds (i8*, i8** @_ZTVN10__cxxabiv120__si_class_type_infoE, i64 2) to i8*), i8* getelementptr inbounds ([6 x i8], [6 x i8]* @_ZTS4Impl, i32 0, i32 0), i8* bitcast ({ i8*, i8* }* @_ZTI9Interface to i8*) }, align 8
+@_ZTV9Interface = linkonce_odr dso_local unnamed_addr constant { [3 x i8*] } { [3 x i8*] [i8* null, i8* bitcast ({ i8*, i8* }* @_ZTI9Interface to i8*), i8* bitcast (void ()* @__cxa_pure_virtual to i8*)] }, align 8
+
+define dso_local void @_Z4Testv() local_unnamed_addr {
+entry:
+  %o = alloca %class.Impl, align 8
+  %0 = bitcast %class.Impl* %o to i8*
+  call void @llvm.lifetime.start.p0i8(i64 16, i8* nonnull %0)
+  call void @_ZN4ImplC2Ev(%class.Impl* nonnull %o)
+  %1 = getelementptr inbounds %class.Impl, %class.Impl* %o, i64 0, i32 0
+  call fastcc void @_ZL11IndirectRunR9Interface(%class.Interface* nonnull dereferenceable(8) %1)
+  call void @llvm.lifetime.end.p0i8(i64 16, i8* nonnull %0)
+  ret void
+}
+
+declare void @llvm.lifetime.start.p0i8(i64 immarg, i8* nocapture)
+
+define linkonce_odr dso_local void @_ZN4ImplC2Ev(%class.Impl* %this) unnamed_addr align 2 {
+entry:
+  %0 = getelementptr %class.Impl, %class.Impl* %this, i64 0, i32 0
+  call void @_ZN9InterfaceC2Ev(%class.Interface* %0)
+  %1 = getelementptr %class.Impl, %class.Impl* %this, i64 0, i32 0, i32 0
+  store i32 (...)** bitcast (i8** getelementptr inbounds ({ [3 x i8*] }, { [3 x i8*] }* @_ZTV4Impl, i64 0, inrange i32 0, i64 2) to i32 (...)**), i32 (...)*** %1, align 8
+  %f = getelementptr inbounds %class.Impl, %class.Impl* %this, i64 0, i32 1
+  store i32 3, i32* %f, align 8
+  ret void
+}
+
+define internal fastcc void @_ZL11IndirectRunR9Interface(%class.Interface* dereferenceable(8) %o) unnamed_addr {
+entry:
+  %0 = bitcast %class.Interface* %o to void (%class.Interface*)***
+  %vtable = load void (%class.Interface*)**, void (%class.Interface*)*** %0, align 8
+  %1 = load void (%class.Interface*)*, void (%class.Interface*)** %vtable, align 8
+  call void %1(%class.Interface* nonnull %o)
+  ret void
+}
+
+declare void @llvm.lifetime.end.p0i8(i64 immarg, i8* nocapture)
+
+define linkonce_odr dso_local void @_ZN9InterfaceC2Ev(%class.Interface* %this) unnamed_addr align 2 {
+entry:
+  %0 = getelementptr %class.Interface, %class.Interface* %this, i64 0, i32 0
+  store i32 (...)** bitcast (i8** getelementptr inbounds ({ [3 x i8*] }, { [3 x i8*] }* @_ZTV9Interface, i64 0, inrange i32 0, i64 2) to i32 (...)**), i32 (...)*** %0, align 8
+  ret void
+}
+
+define linkonce_odr dso_local void @_ZN4Impl3RunEv(%class.Impl* %this) unnamed_addr align 2 {
+entry:
+  %ref.tmp = alloca %class.Impl*, align 8
+  %0 = bitcast %class.Impl** %ref.tmp to i8*

[PATCH] D68049: Propeller: Clang options for basic block sections

[Sriraman Tallam via Phabricator via cfe-commits]

tmsriram added a comment.

In D68049#1870094 , @tmsriram wrote:

> In D68049#1868623 , @MaskRay wrote:
>
> > > In D68049#1865967 , @MaskRay 
> > > wrote:
> > >  If you don't mind, I can push a Diff to this Differential which will 
> > > address these review comments.
> >
> > I can't because I can't figure out the patch relationship...
> >
> > First, this patch does not build on its own. I try applying D68063 
> >  first, then this patch. It still does not 
> > compile..
>


Darn, the include of "llvm/ProfileData/BBSectionsProf.h" is missing in 
BackendUtil.cpp.  Made this mistake when splitting the patch. I will fix this 
shortly.

> Weird, getBBSectionsList is defined by D68063 
> .  Let me try doing that again.  I will also 
> address the rest of your comments.
> 
>> 
>> 
>>   clang/lib/CodeGen/BackendUtil.cpp:484:11: error: no member named 
>> 'propeller' in namespace 'llvm'
>>   llvm::propeller::getBBSectionsList(CodeGenOpts.BBSections,
>> 
>> 
>> Chatted with shenhan and xur offline. I tend to agree that 
>> -fbasicblock-sections=label can improve profile accuracy. It'd be nice to 
>> make that feature separate,
>>  even if there is still a debate on whether the rest of Propeller is done in 
>> a maintainable way.
>> 
>> I think the patch series should probably be structured this way:
>> 
>> 1. LLVM CodeGen: enables basic block sections.
>> 2. clang Driver/Frontend/CodeGen: pass basic block sections options to LLVM.
>> 3. LLVM CodeGen: which enables the rest of Propeller options.
>> 4. lld: a file similar to lld/ELF/LTO.cpp . It should be a thin wrapper of 
>> Propeller features. It should not do hacky diassembly tasks.
>> 5. clang Driver/Frontend/CodeGen: passes compiler/linker options to 3 and 4
>> 
>>   Making 1 and 2 separate can help move forward the patch series. 1 and 2 
>> should not reference `llvm::propeller`.




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

https://reviews.llvm.org/D68049



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

[PATCH] D68049: Propeller: Clang options for basic block sections

[Sriraman Tallam via Phabricator via cfe-commits]

tmsriram added a comment.

In D68049#1868623 , @MaskRay wrote:

> > In D68049#1865967 , @MaskRay wrote:
> >  If you don't mind, I can push a Diff to this Differential which will 
> > address these review comments.
>
> I can't because I can't figure out the patch relationship...
>
> First, this patch does not build on its own. I try applying D68063 
>  first, then this patch. It still does not 
> compile..


Weird, getBBSectionsList is defined by D68063 
.  Let me try doing that again.  I will also 
address the rest of your comments.

> 
> 
>   clang/lib/CodeGen/BackendUtil.cpp:484:11: error: no member named 
> 'propeller' in namespace 'llvm'
>   llvm::propeller::getBBSectionsList(CodeGenOpts.BBSections,
> 
> 
> Chatted with shenhan and xur offline. I tend to agree that 
> -fbasicblock-sections=label can improve profile accuracy. It'd be nice to 
> make that feature separate,
>  even if there is still a debate on whether the rest of Propeller is done in 
> a maintainable way.
> 
> I think the patch series should probably be structured this way:
> 
> 1. LLVM CodeGen: enables basic block sections.
> 2. clang Driver/Frontend/CodeGen: pass basic block sections options to LLVM.
> 3. LLVM CodeGen: which enables the rest of Propeller options.
> 4. lld: a file similar to lld/ELF/LTO.cpp . It should be a thin wrapper of 
> Propeller features. It should not do hacky diassembly tasks.
> 5. clang Driver/Frontend/CodeGen: passes compiler/linker options to 3 and 4
> 
>   Making 1 and 2 separate can help move forward the patch series. 1 and 2 
> should not reference `llvm::propeller`.






Comment at: clang/lib/CodeGen/BackendUtil.cpp:444
+  while ((std::getline(fin, line)).good()) {
+StringRef S(line);
+// Lines beginning with @, # are not useful here.

grimar wrote:
> Something is wrong with the namings (I am not an expert in lib/CodeGen), but 
> you are mixing lower vs upper case styles: "fin", "line", "S", "R". Seems the 
> code around prefers upper case.
I am moving this function out of clang into llvm as this needs to be shared by 
llc, llvm and lld.  I will address all your comments for this function in the 
llvm change.


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

https://reviews.llvm.org/D68049



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

[PATCH] D71110: [clangd] A tool to evaluate cross-file rename.

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev added a comment.

Thanks! Now I can build it.

I was able to get some rename failures through this tool, though :( I had to 
disable the check for number of affected files first, but then this seems to be 
problematic:

  llvm/include/llvm/ADT/ArrayRef.h llvm::ArrayRef clangd
  llvm/include/llvm/ADT/StringRef.h llvm::StringRef clangd


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71110



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

[PATCH] D74414: Use std::foo_t rather than std::foo in clang.

[Benjamin Kramer via Phabricator via cfe-commits]

bkramer accepted this revision.
bkramer added a comment.
This revision is now accepted and ready to land.

lg


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74414



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