[PATCH] D69726: [analyzer] DynamicSize: Store the dynamic size

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

Here's a reduced repro - a file that has different behavior before and after 
the patch (sorry, not perfectly reduced, my creduce is broken again):

  // RUN: clang --analyze %s
  typedef Oid;
  typedef Pointer;
  typedef text;
  errstart(int elevel, filename, lineno, funcname, domain);
  typedef Datum;
  typedef struct {
Oid elemtype;
  } ArrayType;
  
  *guc_malloc(elevel, size) {
void *data;
data = malloc(size);
if (data == (0))
  (errstart(elevel, "c", 3298, __func__, (0))
   ? ((errcode('0') & 0x3F) < 24)
   : 0);
return (errstart(elevel, "c", 3324, __func__, (0)) ? ((errcode(((24) : 
0);
  }
  
  ParseLongOption(*string, *name, value) {
*name = guc_malloc(21, 1);
__builtin___strlcpy_chk(*name, string, 1, string[1]);
  }
  
  ProcessGUCArray(ArrayType *array, action) {
int i = 0;
{ (array)->elemtype) = 25)), "c", 7599); }
for (i = i + 1; char *)(array)) + sizeof(ArrayType)))[0];) {
  Datum d;
  char s;
  char name;
  char value;
  d = array_ref();
  s = text_to_cstring((text)((Pointer)(d)));
  ParseLongOption(s, , );
(errstart(19, "c", 7629, __func__, (0)) ? ((errcode(), errmsg(name)))
: 0);
}
  }

It's most likely some budget heuristic acting up. By slightly changing stuff 
that has shouldn't have any effect you can easily eliminate the regression or 
even reverse it (so that the warning appeared before the patch but not after 
the patch). The patch definitely changes modeling so it's possible that it 
causes budgets to be spent differently.

One particular thing i noticed about the behavior after the patch (by observing 
exploded graph topologies) is that it causes states to be //merged// more 
often. Which is expected because where previously we've created a new extent 
symbol and added a  new constraint, currently we simply re-use the existing 
symbol. This causes 10% improvement in the number of generated nodes. I also 
didn't immediately notice any unintended state splits.

I think we should indeed move on. I'm curious which specific budget do we hit 
but this patch definitely didn't introduce the root cause of the problem, only 
accidentally surfaced it. We should still investigate the tracking bug though, 
i.e. why path in `guc_malloc()` isn't explained.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69726

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

[PATCH] D80344: [Windows SEH]: HARDWARE EXCEPTION HANDLING (MSVC -EHa) - Part 1

[Aaron Smith via Phabricator via cfe-commits]

asmith added a comment.

@rjmccall @rsmith Thanks for your help. Do you have any additional feedback 
before we commit?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80344

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

[PATCH] D80344: [Windows SEH]: HARDWARE EXCEPTION HANDLING (MSVC -EHa) - Part 1

[Aaron Smith via Phabricator via cfe-commits]

asmith added a comment.



In D80344#2671157 , @lebedev.ri wrote:

> It would be good for @rjmccall / @rsmith / etc to actually finish reviewing 
> this and accept it.
> I would personally want to see the next patches - what changes are needed for 
> llvm analysis, transforms?

All changes were provided a year ago and discussed on llvm-dev. I’m inclined to 
move forward unless any of the reviewers have additional comments???


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80344

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

[PATCH] D99810: [ifs][elfabi] Merge llvm-elfabi tool into llvm-ifs

[Haowei Wu via Phabricator via cfe-commits]

haowei marked 3 inline comments as done.
haowei added a comment.

I will try to split this change.




Comment at: clang/lib/Driver/ToolChains/InterfaceStubs.cpp:26
   const bool WriteBin = !Args.getLastArg(options::OPT_emit_merged_ifs);
-  CmdArgs.push_back(WriteBin ? "write-bin" : "write-ifs");
+  CmdArgs.push_back(WriteBin ? "--output-format=ELF" : "--output-format=IFS");
   CmdArgs.push_back("-o");

phosek wrote:
> Do we know whether the binary output format for the current target is ELF? 
> Shouldn't we check it first?
I tried following way:

```
const bool WriteBin = !Args.getLastArg(options::OPT_emit_merged_ifs);
  if (WriteBin) {
llvm::Triple::OSType OS = 
C.getDefaultToolChain().getEffectiveTriple().getOS();
switch(OS) {
case llvm::Triple::OSType::DragonFly:
case llvm::Triple::OSType::Fuchsia:
case llvm::Triple::OSType::KFreeBSD:
case llvm::Triple::OSType::Linux:
case llvm::Triple::OSType::Lv2:
case llvm::Triple::OSType::NetBSD:
case llvm::Triple::OSType::OpenBSD:
case llvm::Triple::OSType::Solaris:
case llvm::Triple::OSType::Haiku:
case llvm::Triple::OSType::Minix:
case llvm::Triple::OSType::NaCl:
case llvm::Triple::OSType::PS4:
case llvm::Triple::OSType::ELFIAMCU:
case llvm::Triple::OSType::Contiki:
case llvm::Triple::OSType::Hurd:
  CmdArgs.push_back("--output-format=ELF");
  break;
// default:
//   // Not adding "--output-format" will cause llvm-ifs to crash.
}
  } else {
CmdArgs.push_back("--output-format=IFS");
  }
```
but it does not work. It looks like the Toolchain object is not properly 
constructed in this stage, which makes it difficult to determine the target of 
current clang invocation. Do you have any suggestions?



Comment at: llvm/include/llvm/InterfaceStub/IFSStub.h:28-52
+enum class IFSSymbolType {
   NoType = ELF::STT_NOTYPE,
   Object = ELF::STT_OBJECT,
   Func = ELF::STT_FUNC,
   TLS = ELF::STT_TLS,
 
   // Type information is 4 bits, so 16 is safely out of range.

phosek wrote:
> Since IFS is supposed to be binary format agnostic, it's a bit confusing to 
> use ELF constants here. Could we instead define these values ourselves and 
> convert them to the corresponding ELF values when generating the stub?
I removed ELF constants and added helper functions.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99810

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

[PATCH] D99683: [HIP] Support ThinLTO

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added a comment.

In D99683#2672668 , @tejohnson wrote:

> In D99683#2672578 , @yaxunl wrote:
>
>> In D99683#2672554 , @tejohnson 
>> wrote:
>>
>>> This raises some higher level questions for me:
>>>
>>> First, how will you deal with other corner cases that won't or cannot be 
>>> imported right now? While enabling importing of noinline functions and 
>>> cranking up the threshold will get the majority of functions imported, 
>>> there are cases that we still won't import (functions/vars that are 
>>> interposable, certain funcs/vars that cannot be renamed, most non-const 
>>> variables with non-trivial initializers).
>>
>> We will document the limitation of thinLTO support of HIP toolchain and 
>> recommend users not to use thinLTO in those corner cases.
>>
>>> Second, force importing of everything transitively referenced defeats the 
>>> purpose of ThinLTO and would probably make it worse than regular LTO. The 
>>> main entry module will need to import everything transitively referenced 
>>> from there, so everything not dead in the binary, which should make that 
>>> module post importing equivalent to a regular LTO module. In addition, 
>>> every other module needs to transitively import everything referenced from 
>>> those modules, making them very large depending on how many leaf vs 
>>> non-leaf functions and variables they contain. What is the goal of doing 
>>> ThinLTO in this case?
>>
>> The objective is to improve optimization/codegen time by using multi-threads 
>> of thinLTO. For example, I have 10 modules each containing a kernel. In full 
>> LTO linking, I get one big module containing 10 kernels with all functions 
>> inlined, and I have one thread for optimization/codegen. With thinLTO, I get 
>> one kernel in each module, with all functions inlined. AMDGPU 
>> internalization and global DCE will remove functions not used by that kernel 
>> in each module. I will get 10 threads, each doing optimization/codegen for 
>> one kernel. Theoretically, there could be 10 times speed up.
>
> That will work as long as there are no dependence edges anywhere between the 
> kernels. Is this a library that has a bunch of totally independent kernels 
> only called externally?

There are no dependence edges between the kernels since they cannot call each 
other. The HIP device compilation output is always a shared library which 
contains multiple independent kernels which can be launched by a HIP program.


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

https://reviews.llvm.org/D99683

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

[PATCH] D98193: [CUDA][HIP] Allow non-ODR use of host var in device

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl updated this revision to Diff 335702.
yaxunl added a comment.

revised by Richard's comments. Check function-scope static var.


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

https://reviews.llvm.org/D98193

Files:
  clang/lib/Sema/SemaExpr.cpp
  clang/test/CodeGenCUDA/device-use-host-var.cu
  clang/test/SemaCUDA/device-use-host-var.cu

Index: clang/test/SemaCUDA/device-use-host-var.cu
===
--- clang/test/SemaCUDA/device-use-host-var.cu
+++ clang/test/SemaCUDA/device-use-host-var.cu
@@ -5,37 +5,96 @@
 
 #include "Inputs/cuda.h"
 
-int global_host_var;
+struct A {
+  int x;
+  static int host_var;
+};
+
+int A::host_var;
+
+namespace X {
+  int host_var;
+}
+
+static int static_host_var;
+
 __device__ int global_dev_var;
 __constant__ int global_constant_var;
 __shared__ int global_shared_var;
-constexpr int global_constexpr_var = 1;
+
+int global_host_var;
 const int global_const_var = 1;
+constexpr int global_constexpr_var = 1;
+
+int global_host_array[2] = {1, 2};
+const int global_const_array[2] = {1, 2};
+constexpr int global_constexpr_array[2] = {1, 2};
+
+A global_host_struct_var{1};
+const A global_const_struct_var{1};
+constexpr A global_constexpr_struct_var{1};
 
 template
 __global__ void kernel(F f) { f(); } // dev-note2 {{called by 'kernel<(lambda}}
 
 __device__ void dev_fun(int *out) {
-  int _host_var = global_host_var; // dev-error {{reference to __host__ variable 'global_host_var' in __device__ function}}
+  // Check access device variables are allowed.
   int _dev_var = global_dev_var;
   int _constant_var = global_constant_var;
   int _shared_var = global_shared_var;
-  const int _constexpr_var = global_constexpr_var;
-  const int _const_var = global_const_var;
-
-  *out = global_host_var; // dev-error {{reference to __host__ variable 'global_host_var' in __device__ function}}
+  *out = ref_dev_var;
+  *out = ref_constant_var;
+  *out = ref_shared_var;
   *out = global_dev_var;
   *out = global_constant_var;
   *out = global_shared_var;
-  *out = global_constexpr_var;
+
+  // Check access of non-const host variables are not allowed.
+  *out = global_host_var; // dev-error {{reference to __host__ variable 'global_host_var' in __device__ function}}
   *out = global_const_var;
+  *out = global_constexpr_var;
+  global_host_var = 1; // dev-error {{reference to __host__ variable 'global_host_var' in __device__ function}}
 
+  // Check reference of non-constexpr host variables are not allowed.
+  int _host_var = global_host_var; // dev-error {{reference to __host__ variable 'global_host_var' in __device__ function}}
+  const int _const_var = global_const_var; // dev-error {{reference to __host__ variable 'global_const_var' in __device__ function}}
+  const int _constexpr_var = global_constexpr_var;
   *out = ref_host_var;
-  *out = ref_dev_var;
-  *out = ref_constant_var;
-  *out = ref_shared_var;
   *out = ref_constexpr_var;
   *out = ref_const_var;
+
+  // Check access member of non-constexpr struct type host variable is not allowed.
+  *out = global_host_struct_var.x; // dev-error {{reference to __host__ variable 'global_host_struct_var' in __device__ function}}
+  *out = global_const_struct_var.x; // dev-error {{reference to __host__ variable 'global_const_struct_var' in __device__ function}}
+  *out = global_constexpr_struct_var.x;
+  global_host_struct_var.x = 1; // dev-error {{reference to __host__ variable 'global_host_struct_var' in __device__ function}}
+
+  // Check address taking of non-constexpr host variables is not allowed.
+  int *p = _host_var; // dev-error {{reference to __host__ variable 'global_host_var' in __device__ function}}
+  const int *cp = _const_var; // dev-error {{reference to __host__ variable 'global_const_var' in __device__ function}}
+  const int *cp2 = _constexpr_var;
+
+  // Check access elements of non-constexpr host array is not allowed.
+  *out = global_host_array[1]; // dev-error {{reference to __host__ variable 'global_host_array' in __device__ function}}
+  *out = global_const_array[1]; // dev-error {{reference to __host__ variable 'global_const_array' in __device__ function}}
+  *out = global_constexpr_array[1];
+
+  // Check ODR-use of host variables in namespace is not allowed.
+  *out = X::host_var; // dev-error {{reference to __host__ variable 'host_var' in __device__ function}}
+
+  // Check ODR-use of static host varables in class or file scope is not allowed.
+  *out = A::host_var; // dev-error {{reference to __host__ variable 'host_var' in __device__ function}}
+  *out = static_host_var; // dev-error {{reference to __host__ variable 'static_host_var' in __device__ function}}
+
+  // Check function-scope static variable is allowed.
+  static int static_var;
+  *out = static_var;
+
+  // Check non-ODR use of host varirables are allowed.
+  *out = sizeof(global_host_var);
+  *out = sizeof(global_host_struct_var.x);
+  decltype(global_host_var) var1;
+  

[PATCH] D99551: [clang-offload-wrapper] Add standard notes for ELF offload images

[George Rokos via Phabricator via cfe-commits]

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

Change looks good, so it's accepted on my end. I'll let the other reviewers 
have a look and post their comments. Please do not commit until we have reached 
an agreement for all 4 patches together (D99551 
, D99552 , 
D99553 , D99612 
).


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

https://reviews.llvm.org/D99551

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

[PATCH] D98912: Fix -Winteger-overflow to diagnose regardless of the top-level syntactic form.

[Volodymyr Sapsai via Phabricator via cfe-commits]

vsapsai accepted this revision.
vsapsai added inline comments.
This revision is now accepted and ready to land.



Comment at: clang/lib/AST/ExprConstant.cpp:972-973
 
+// Determine if we might warn that the given expression exhibits undefined
+// behavior.
+bool mightWarnOnUndefinedBehavior(const Expr *E) {

rsmith wrote:
> vsapsai wrote:
> > I'm not entirely sure but it can be useful to make it explicit in the 
> > comment the method doesn't deal with all the subexpressions recursively. 
> > Not convinced it is useful because don't have a good wording and you can 
> > get that information from the short method implementation. It is possible 
> > my suggestion is caused by checking the patch top-to-bottom without seeing 
> > how `mightWarnOnUndefinedBehavior` is used first, but that's not how 
> > developers would read the code later.
> Would renaming this to something like 
> `shouldEvaluateForUndefinedBehaviorChecks` help? That still doesn't 
> completely capture the non-recursive nature. I think improving the comment is 
> a good idea regardless.
Don't have strong feelings about `shouldEvaluateForUndefinedBehaviorChecks`. 
Both `shouldEvaluateForUndefinedBehaviorChecks` and 
`mightWarnOnUndefinedBehavior` convey enough intention and actual 
implementation is fairly easy to understand. And information at the call site 
in `ShouldEvaluate` seems to be sufficient for intention and implementation 
purposes.

Also considered inlining `mightWarnOnUndefinedBehavior` but it doesn't look 
like a better alternative.

It's up to you if you want to tweak it further, I don't have good suggestions.



Comment at: clang/lib/Sema/SemaExpr.cpp:7125-7126
   << DestTy;
-  return CK_IntegralCast;
+  Src = ExprError();
+  return CK_Dependent;
 case Type::STK_CPointer:

rsmith wrote:
> vsapsai wrote:
> > What is the purpose of these changes to return `CK_Dependent`? Tests are 
> > passing without them and it's not obvious to me why do you need to change 
> > `CK_IntegralCast` to `CK_Dependent`.
> It shouldn't matter what we return, because the caller should always bail out 
> and ignore our return value if `Src` is set to an invalid expression. But in 
> principle, `CK_IntegralCast` is wrong, because this situation doesn't meet 
> the constraints for an integral cast. We use "dependent" to mean not only 
> "dependent on a C++ template parameter" but also "dependent on an error", so 
> that seemed like the least wrong cast kind for this situation.
Thanks for the explanation. "dependent on an error" is an important piece of 
information and now it makes more sense.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98912

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

[PATCH] D93377: [Clang] Add __ibm128 type to represent ppc_fp128

[Qiu Chaofan via Phabricator via cfe-commits]

qiucf added a comment.

Ping.. Any further comments?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93377

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

[PATCH] D100000: [clang] WIP: Implement simpler alternative to two-phase lookup for NRVO

[Matheus Izvekov via Phabricator via cfe-commits]

mizvekov updated this revision to Diff 335689.
mizvekov added a comment.

.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D10

Files:
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/SemaCoroutine.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/lib/Sema/SemaStmt.cpp

Index: clang/lib/Sema/SemaStmt.cpp
===
--- clang/lib/Sema/SemaStmt.cpp
+++ clang/lib/Sema/SemaStmt.cpp
@@ -3046,25 +3046,22 @@
 /// \returns An aggregate which contains the Candidate and isMoveEligible
 /// and isCopyElidable methods. If Candidate is non-null, it means
 /// isMoveEligible() would be true under the most permissive language standard.
-Sema::NRVOResult Sema::getNRVOResult(const VarDecl *VD, bool Parenthesized,
- bool ForceCXX20) {
+Sema::NRVOResult Sema::getNRVOResult(const VarDecl *VD, bool Parenthesized) {
   if (!VD)
 return NRVOResult();
 
-  bool hasCXX11 = getLangOpts().CPlusPlus11 || ForceCXX20,
-   hasCXX20 = getLangOpts().CPlusPlus20 || ForceCXX20;
   NRVOResult Res{VD, NRVOResult::MoveEligibleAndCopyElidable, Parenthesized};
 
   // (other than a function ... parameter)
   if (VD->getKind() == Decl::ParmVar) {
-downgradeNRVOResult(Res, hasCXX11);
+downgradeNRVOResult(Res, getLangOpts().CPlusPlus11);
   } else if (VD->getKind() != Decl::Var) {
 return NRVOResult();
   }
 
   // (other than ... a catch-clause parameter)
   if (VD->isExceptionVariable())
-downgradeNRVOResult(Res, hasCXX20);
+downgradeNRVOResult(Res, getLangOpts().CPlusPlus20);
 
   // ...automatic...
   if (!VD->hasLocalStorage())
@@ -3090,7 +3087,7 @@
 if (VDReferencedType.isVolatileQualified() ||
 !VDReferencedType->isObjectType())
   return NRVOResult();
-downgradeNRVOResult(Res, hasCXX20);
+downgradeNRVOResult(Res, getLangOpts().CPlusPlus20);
   } else {
 return NRVOResult();
   }
@@ -3099,7 +3096,7 @@
   // alignment cannot use NRVO.
   if (!VDType->isDependentType() && VD->hasAttr() &&
   Context.getDeclAlign(VD) > Context.getTypeAlignInChars(VDType))
-downgradeNRVOResult(Res, hasCXX11);
+downgradeNRVOResult(Res, getLangOpts().CPlusPlus11);
 
   return Res;
 }
@@ -3118,7 +3115,7 @@
 /// \returns An aggregate which contains the Candidate and isMoveEligible
 /// and isCopyElidable methods. If Candidate is non-null, it means
 /// isMoveEligible() would be true under the most permissive language standard.
-Sema::NRVOResult Sema::getNRVOResult(Expr *, bool ForceCXX2b) {
+Sema::NRVOResult Sema::getNRVOResult(Expr *) {
   if (!E)
 return NRVOResult();
   bool Parenthesized = isa(E);
@@ -3128,12 +3125,39 @@
   if (!DR || DR->refersToEnclosingVariableOrCapture())
 return NRVOResult();
   const auto *VD = dyn_cast(DR->getDecl());
-  NRVOResult Res = getNRVOResult(VD, Parenthesized, /*ForceCXX20=*/ForceCXX2b);
-  if (Res.Candidate && E->getValueKind() != VK_XValue &&
-  (ForceCXX2b || getLangOpts().CPlusPlus2b)) {
-E = ImplicitCastExpr::Create(Context, VD->getType().getNonReferenceType(),
- CK_NoOp, E, nullptr, VK_XValue,
- FPOptionsOverride());
+  NRVOResult Res = getNRVOResult(VD, Parenthesized);
+  if (Res.S >= NRVOResult::MoveEligible) {
+ExprValueKind CastToKind =
+getLangOpts().CPlusPlus2b ? VK_XValue : VK_XValue;
+if (E->getValueKind() != CastToKind) {
+  E = ImplicitCastExpr::Create(Context, VD->getType().getNonReferenceType(),
+   CK_NoOp, E, nullptr, CastToKind,
+   FPOptionsOverride());
+}
+  } else if (Res.Candidate &&
+ !getDiagnostics().isIgnored(diag::warn_return_std_move,
+ E->getExprLoc())) {
+QualType QT = Res.Candidate->getType();
+if (QT->isLValueReferenceType()) {
+  // Adding 'std::move' around an lvalue reference variable's name is
+  // dangerous. Don't suggest it.
+} else if (QT.getNonReferenceType()
+   .getUnqualifiedType()
+   .isTriviallyCopyableType(Context)) {
+  // Adding 'std::move' around a trivially copyable variable is probably
+  // pointless. Don't suggest it.
+} else {
+  bool IsThrow =
+  false; //(Entity.getKind() == InitializedEntity::EK_Exception);
+  SmallString<32> Str;
+  Str += "std::move(";
+  Str += Res.Candidate->getDeclName().getAsString();
+  Str += ")";
+  Diag(E->getExprLoc(), diag::warn_return_std_move)
+  << E->getSourceRange() << Res.Candidate->getDeclName() << IsThrow;
+  Diag(E->getExprLoc(), diag::note_add_std_move)
+  << FixItHint::CreateReplacement(E->getSourceRange(), Str);
+}
   }
   return Res;
 }
@@ -3185,160 +3209,6 @@
 downgradeNRVOResult(Res, getLangOpts().CPlusPlus11);
 }
 
-/// Try to perform the 

[PATCH] D100000: [clang] WIP: Implement simpler alternative to two-phase lookup for NRVO

[Matheus Izvekov via Phabricator via cfe-commits]

mizvekov updated this revision to Diff 335688.
mizvekov added a comment.

.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D10

Files:
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/SemaCoroutine.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/lib/Sema/SemaStmt.cpp

Index: clang/lib/Sema/SemaStmt.cpp
===
--- clang/lib/Sema/SemaStmt.cpp
+++ clang/lib/Sema/SemaStmt.cpp
@@ -3118,7 +3118,7 @@
 /// \returns An aggregate which contains the Candidate and isMoveEligible
 /// and isCopyElidable methods. If Candidate is non-null, it means
 /// isMoveEligible() would be true under the most permissive language standard.
-Sema::NRVOResult Sema::getNRVOResult(Expr *, bool ForceCXX2b) {
+Sema::NRVOResult Sema::getNRVOResult(Expr *) {
   if (!E)
 return NRVOResult();
   bool Parenthesized = isa(E);
@@ -3128,12 +3128,38 @@
   if (!DR || DR->refersToEnclosingVariableOrCapture())
 return NRVOResult();
   const auto *VD = dyn_cast(DR->getDecl());
-  NRVOResult Res = getNRVOResult(VD, Parenthesized, /*ForceCXX20=*/ForceCXX2b);
-  if (Res.Candidate && E->getValueKind() != VK_XValue &&
-  (ForceCXX2b || getLangOpts().CPlusPlus2b)) {
-E = ImplicitCastExpr::Create(Context, VD->getType().getNonReferenceType(),
- CK_NoOp, E, nullptr, VK_XValue,
- FPOptionsOverride());
+  NRVOResult Res = getNRVOResult(VD, Parenthesized);
+  if (Res.S >= NRVOResult::MoveEligible) {
+ExprValueKind CastToKind = VK_XValue;
+if (E->getValueKind() != CastToKind) {
+  E = ImplicitCastExpr::Create(Context, VD->getType().getNonReferenceType(),
+   CK_NoOp, E, nullptr, CastToKind,
+   FPOptionsOverride());
+}
+  } else if (Res.Candidate &&
+ !getDiagnostics().isIgnored(diag::warn_return_std_move,
+ E->getExprLoc())) {
+QualType QT = Res.Candidate->getType();
+if (QT->isLValueReferenceType()) {
+  // Adding 'std::move' around an lvalue reference variable's name is
+  // dangerous. Don't suggest it.
+} else if (QT.getNonReferenceType()
+   .getUnqualifiedType()
+   .isTriviallyCopyableType(Context)) {
+  // Adding 'std::move' around a trivially copyable variable is probably
+  // pointless. Don't suggest it.
+} else {
+  bool IsThrow =
+  false; //(Entity.getKind() == InitializedEntity::EK_Exception);
+  SmallString<32> Str;
+  Str += "std::move(";
+  Str += Res.Candidate->getDeclName().getAsString();
+  Str += ")";
+  Diag(E->getExprLoc(), diag::warn_return_std_move)
+  << E->getSourceRange() << Res.Candidate->getDeclName() << IsThrow;
+  Diag(E->getExprLoc(), diag::note_add_std_move)
+  << FixItHint::CreateReplacement(E->getSourceRange(), Str);
+}
   }
   return Res;
 }
@@ -3185,160 +3211,6 @@
 downgradeNRVOResult(Res, getLangOpts().CPlusPlus11);
 }
 
-/// Try to perform the initialization of a potentially-movable value,
-/// which is the operand to a return or throw statement.
-///
-/// This routine implements C++20 [class.copy.elision]p3, which attempts to
-/// treat returned lvalues as rvalues in certain cases (to prefer move
-/// construction), then falls back to treating them as lvalues if that failed.
-///
-/// \param ConvertingConstructorsOnly If true, follow [class.copy.elision]p3 and
-/// reject resolutions that find non-constructors, such as derived-to-base
-/// conversions or `operator T()&&` member functions. If false, do consider such
-/// conversion sequences.
-///
-/// \param Res We will fill this in if move-initialization was possible.
-/// If move-initialization is not possible, such that we must fall back to
-/// treating the operand as an lvalue, we will leave Res in its original
-/// invalid state.
-///
-/// \returns Whether we need to do the second overload resolution. If the first
-/// overload resolution fails, or if the first overload resolution succeeds but
-/// the selected constructor/operator doesn't match the additional criteria, we
-/// need to do the second overload resolution.
-static bool TryMoveInitialization(Sema , const InitializedEntity ,
-  const VarDecl *NRVOCandidate, Expr *,
-  bool ConvertingConstructorsOnly,
-  bool IsDiagnosticsCheck, ExprResult ) {
-  ImplicitCastExpr AsRvalue(ImplicitCastExpr::OnStack, Value->getType(),
-CK_NoOp, Value, VK_XValue, FPOptionsOverride());
-
-  Expr *InitExpr = 
-
-  InitializationKind Kind = InitializationKind::CreateCopy(
-  Value->getBeginLoc(), Value->getBeginLoc());
-
-  InitializationSequence Seq(S, Entity, Kind, InitExpr);
-
-  bool NeedSecondOverloadResolution = true;

[clang] 4018268 - Add missing CHECK lines in test

[via cfe-commits]

Author: Weverything
Date: 2021-04-06T18:00:31-07:00
New Revision: 401826800ef1d2e73ac9ea8e7e31d6c12d543c5e

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

LOG: Add missing CHECK lines in test

Added: 


Modified: 
clang/test/SemaCXX/warn-max-unsigned-zero.cpp

Removed: 




diff  --git a/clang/test/SemaCXX/warn-max-unsigned-zero.cpp 
b/clang/test/SemaCXX/warn-max-unsigned-zero.cpp
index 5564b101d8ec..7d3c74bb08a4 100644
--- a/clang/test/SemaCXX/warn-max-unsigned-zero.cpp
+++ b/clang/test/SemaCXX/warn-max-unsigned-zero.cpp
@@ -15,18 +15,18 @@ void test(unsigned u) {
   auto b = std::max(u, 0u);
   // expected-warning@-1{{taking the max of a value and unsigned zero is 
always equal to the other value}}
   // expected-note@-2{{remove call to max function and unsigned zero argument}}
-  // 
fix-it:"/usr/local/google/home/rtrieu/clang/open/llvm/tools/clang/test/SemaCXX/warn-max-unsigned-zero.cpp":{13:12-13:20}:""
-  // 
fix-it:"/usr/local/google/home/rtrieu/clang/open/llvm/tools/clang/test/SemaCXX/warn-max-unsigned-zero.cpp":{13:22-13:26}:""
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:12-[[@LINE-3]]:20}:""
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-4]]:22-[[@LINE-4]]:26}:""
   auto c = std::max(0u, 55u);
   // expected-warning@-1{{taking the max of unsigned zero and a value is 
always equal to the other value}}
   // expected-note@-2{{remove call to max function and unsigned zero argument}}
-  // 
fix-it:"/usr/local/google/home/rtrieu/clang/open/llvm/tools/clang/test/SemaCXX/warn-max-unsigned-zero.cpp":{16:12-16:20}:""
-  // 
fix-it:"/usr/local/google/home/rtrieu/clang/open/llvm/tools/clang/test/SemaCXX/warn-max-unsigned-zero.cpp":{16:21-16:24}:""
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:12-[[@LINE-3]]:20}:""
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-4]]:21-[[@LINE-4]]:24}:""
   auto d = std::max(0u, u);
   // expected-warning@-1{{taking the max of unsigned zero and a value is 
always equal to the other value}}
   // expected-note@-2{{remove call to max function and unsigned zero argument}}
-  // 
fix-it:"/usr/local/google/home/rtrieu/clang/open/llvm/tools/clang/test/SemaCXX/warn-max-unsigned-zero.cpp":{19:12-19:20}:""
-  // 
fix-it:"/usr/local/google/home/rtrieu/clang/open/llvm/tools/clang/test/SemaCXX/warn-max-unsigned-zero.cpp":{19:21-19:24}:""
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:12-[[@LINE-3]]:20}:""
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-4]]:21-[[@LINE-4]]:24}:""
 }
 
 void negative_test(signed s) {
@@ -41,8 +41,8 @@ unsigned template_test() {
   return std::max(x, 0u);
   // expected-warning@-1{{taking the max of a value and unsigned zero is 
always equal to the other value}}
   // expected-note@-2{{remove call to max function and unsigned zero argument}}
-  // 
fix-it:"/usr/local/google/home/rtrieu/clang/open/llvm/tools/clang/test/SemaCXX/warn-max-unsigned-zero.cpp":{33:10-33:18}:""
-  // 
fix-it:"/usr/local/google/home/rtrieu/clang/open/llvm/tools/clang/test/SemaCXX/warn-max-unsigned-zero.cpp":{33:20-33:24}:""
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:10-[[@LINE-3]]:18}:""
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-4]]:20-[[@LINE-4]]:24}:""
 }
 
 int a = template_test<0>() + template_test<1>() + template_test<2>();



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

[clang] 86175d5 - Minor fix for test hip-code-object-version.hip

[Yaxun Liu via cfe-commits]

Author: Yaxun (Sam) Liu
Date: 2021-04-06T20:32:16-04:00
New Revision: 86175d5fedba7c09ad09ee5afd359e7f9246367a

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

LOG: Minor fix for test hip-code-object-version.hip

Changed the order of checking of v2 and v3.

Change-Id: Ifea8197b398afdfb0aa1bd40140cda30f00f0c17

Added: 


Modified: 
clang/test/Driver/hip-code-object-version.hip

Removed: 




diff  --git a/clang/test/Driver/hip-code-object-version.hip 
b/clang/test/Driver/hip-code-object-version.hip
index ba73ace411b5..0e85d9008602 100644
--- a/clang/test/Driver/hip-code-object-version.hip
+++ b/clang/test/Driver/hip-code-object-version.hip
@@ -1,5 +1,21 @@
 // REQUIRES: clang-driver, amdgpu-registered-target
 
+// Check bundle ID for code object v2.
+
+// RUN: %clang -### -target x86_64-linux-gnu \
+// RUN:   -mno-code-object-v3 \
+// RUN:   --offload-arch=gfx906 -nogpulib \
+// RUN:   %s 2>&1 | FileCheck -check-prefixes=V2,V2-WARN %s
+
+// RUN: %clang -### -target x86_64-linux-gnu \
+// RUN:   -mcode-object-version=2 \
+// RUN:   --offload-arch=gfx906 -nogpulib \
+// RUN:   %s 2>&1 | FileCheck -check-prefix=V2 %s
+
+// V2-WARN: warning: argument '-mno-code-object-v3' is deprecated, use 
'-mcode-object-version=2' instead [-Wdeprecated]
+// V2: "-mllvm" "--amdhsa-code-object-version=2"
+// V2: "-targets=host-x86_64-unknown-linux,hip-amdgcn-amd-amdhsa--gfx906"
+
 // Check bundle ID for code object v3.
 
 // RUN: %clang -### -target x86_64-linux-gnu \
@@ -21,22 +37,6 @@
 // V3: "-mllvm" "--amdhsa-code-object-version=3"
 // V3: "-targets=host-x86_64-unknown-linux,hip-amdgcn-amd-amdhsa--gfx906"
 
-// Check bundle ID for code object v2.
-
-// RUN: %clang -### -target x86_64-linux-gnu \
-// RUN:   -mno-code-object-v3 \
-// RUN:   --offload-arch=gfx906 -nogpulib \
-// RUN:   %s 2>&1 | FileCheck -check-prefixes=V2,V2-WARN %s
-
-// RUN: %clang -### -target x86_64-linux-gnu \
-// RUN:   -mcode-object-version=2 \
-// RUN:   --offload-arch=gfx906 -nogpulib \
-// RUN:   %s 2>&1 | FileCheck -check-prefix=V2 %s
-
-// V2-WARN: warning: argument '-mno-code-object-v3' is deprecated, use 
'-mcode-object-version=2' instead [-Wdeprecated]
-// V2: "-mllvm" "--amdhsa-code-object-version=2"
-// V2: "-targets=host-x86_64-unknown-linux,hip-amdgcn-amd-amdhsa--gfx906"
-
 // Check bundle ID for code object version 4.
 
 // RUN: %clang -### -target x86_64-linux-gnu \



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

[PATCH] D99235: [HIP] Change to code object v4

[Yaxun Liu via Phabricator via cfe-commits]

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



Comment at: clang/test/Driver/hip-code-object-version.hip:24-39
 // Check bundle ID for code object v2.
 
 // RUN: %clang -### -target x86_64-linux-gnu \
 // RUN:   -mno-code-object-v3 \
 // RUN:   --offload-arch=gfx906 -nogpulib \
 // RUN:   %s 2>&1 | FileCheck -check-prefixes=V2,V2-WARN %s
 

tra wrote:
> Nit: it would be nice to move V2 tests above the V3, so the tests are in 
> order. 
sorry I missed it. will do.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99235

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

[PATCH] D100000: [clang] WIP: Implement simpler alternative to two-phase lookup for NRVO WIP, not ready for review.

[Matheus Izvekov via Phabricator via cfe-commits]

mizvekov created this revision.
Herald added a subscriber: lxfind.
mizvekov requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Signed-off-by: Matheus Izvekov 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D10

Files:
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/SemaCoroutine.cpp
  clang/lib/Sema/SemaStmt.cpp

Index: clang/lib/Sema/SemaStmt.cpp
===
--- clang/lib/Sema/SemaStmt.cpp
+++ clang/lib/Sema/SemaStmt.cpp
@@ -3118,7 +3118,7 @@
 /// \returns An aggregate which contains the Candidate and isMoveEligible
 /// and isCopyElidable methods. If Candidate is non-null, it means
 /// isMoveEligible() would be true under the most permissive language standard.
-Sema::NRVOResult Sema::getNRVOResult(Expr *, bool ForceCXX2b) {
+Sema::NRVOResult Sema::getNRVOResult(Expr *) {
   if (!E)
 return NRVOResult();
   bool Parenthesized = isa(E);
@@ -3128,12 +3128,38 @@
   if (!DR || DR->refersToEnclosingVariableOrCapture())
 return NRVOResult();
   const auto *VD = dyn_cast(DR->getDecl());
-  NRVOResult Res = getNRVOResult(VD, Parenthesized, /*ForceCXX20=*/ForceCXX2b);
-  if (Res.Candidate && E->getValueKind() != VK_XValue &&
-  (ForceCXX2b || getLangOpts().CPlusPlus2b)) {
-E = ImplicitCastExpr::Create(Context, VD->getType().getNonReferenceType(),
- CK_NoOp, E, nullptr, VK_XValue,
- FPOptionsOverride());
+  NRVOResult Res = getNRVOResult(VD, Parenthesized);
+  if (Res.S >= NRVOResult::MoveEligible) {
+ExprValueKind CastToKind = VK_XValue;
+if (E->getValueKind() != CastToKind) {
+  E = ImplicitCastExpr::Create(Context, VD->getType().getNonReferenceType(),
+   CK_NoOp, E, nullptr, CastToKind,
+   FPOptionsOverride());
+}
+  } else if (Res.Candidate &&
+ !getDiagnostics().isIgnored(diag::warn_return_std_move,
+ E->getExprLoc())) {
+QualType QT = Res.Candidate->getType();
+if (QT->isLValueReferenceType()) {
+  // Adding 'std::move' around an lvalue reference variable's name is
+  // dangerous. Don't suggest it.
+} else if (QT.getNonReferenceType()
+   .getUnqualifiedType()
+   .isTriviallyCopyableType(Context)) {
+  // Adding 'std::move' around a trivially copyable variable is probably
+  // pointless. Don't suggest it.
+} else {
+  bool IsThrow =
+  false; //(Entity.getKind() == InitializedEntity::EK_Exception);
+  SmallString<32> Str;
+  Str += "std::move(";
+  Str += Res.Candidate->getDeclName().getAsString();
+  Str += ")";
+  Diag(E->getExprLoc(), diag::warn_return_std_move)
+  << E->getSourceRange() << Res.Candidate->getDeclName() << IsThrow;
+  Diag(E->getExprLoc(), diag::note_add_std_move)
+  << FixItHint::CreateReplacement(E->getSourceRange(), Str);
+}
   }
   return Res;
 }
@@ -3185,160 +3211,6 @@
 downgradeNRVOResult(Res, getLangOpts().CPlusPlus11);
 }
 
-/// Try to perform the initialization of a potentially-movable value,
-/// which is the operand to a return or throw statement.
-///
-/// This routine implements C++20 [class.copy.elision]p3, which attempts to
-/// treat returned lvalues as rvalues in certain cases (to prefer move
-/// construction), then falls back to treating them as lvalues if that failed.
-///
-/// \param ConvertingConstructorsOnly If true, follow [class.copy.elision]p3 and
-/// reject resolutions that find non-constructors, such as derived-to-base
-/// conversions or `operator T()&&` member functions. If false, do consider such
-/// conversion sequences.
-///
-/// \param Res We will fill this in if move-initialization was possible.
-/// If move-initialization is not possible, such that we must fall back to
-/// treating the operand as an lvalue, we will leave Res in its original
-/// invalid state.
-///
-/// \returns Whether we need to do the second overload resolution. If the first
-/// overload resolution fails, or if the first overload resolution succeeds but
-/// the selected constructor/operator doesn't match the additional criteria, we
-/// need to do the second overload resolution.
-static bool TryMoveInitialization(Sema , const InitializedEntity ,
-  const VarDecl *NRVOCandidate, Expr *,
-  bool ConvertingConstructorsOnly,
-  bool IsDiagnosticsCheck, ExprResult ) {
-  ImplicitCastExpr AsRvalue(ImplicitCastExpr::OnStack, Value->getType(),
-CK_NoOp, Value, VK_XValue, FPOptionsOverride());
-
-  Expr *InitExpr = 
-
-  InitializationKind Kind = InitializationKind::CreateCopy(
-  Value->getBeginLoc(), Value->getBeginLoc());
-
-  InitializationSequence Seq(S, Entity, Kind, InitExpr);
-
-  

[PATCH] D99235: [HIP] Change to code object v4

[Yaxun Liu via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
yaxunl marked an inline comment as done.
Closed by commit rG4fd05e0ad7fb: [HIP] Change to code object v4 (authored by 
yaxunl).
Herald added a project: clang.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99235

Files:
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/lib/Driver/ToolChains/HIP.cpp
  clang/test/Driver/hip-code-object-version.hip
  clang/test/Driver/hip-target-id.hip
  clang/test/Driver/hip-toolchain-device-only.hip
  clang/test/Driver/hip-toolchain-no-rdc.hip
  clang/test/Driver/hip-toolchain-rdc-separate.hip
  clang/test/Driver/hip-toolchain-rdc-static-lib.hip
  clang/test/Driver/hip-toolchain-rdc.hip
  clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp

Index: clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp
===
--- clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp
+++ clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp
@@ -1143,6 +1143,7 @@
  .Case("host", true)
  .Case("openmp", true)
  .Case("hip", true)
+ .Case("hipv4", true)
  .Default(false);
 
 bool TripleIsValid = !Triple.empty();
Index: clang/test/Driver/hip-toolchain-rdc.hip
===
--- clang/test/Driver/hip-toolchain-rdc.hip
+++ clang/test/Driver/hip-toolchain-rdc.hip
@@ -97,7 +97,7 @@
 // combine images generated into hip fat binary object
 // CHECK: [[BUNDLER:".*clang-offload-bundler"]] "-type=o"
 // CHECK-SAME: "-bundle-align=4096"
-// CHECK-SAME: "-targets={{.*}},hip-amdgcn-amd-amdhsa--gfx803,hip-amdgcn-amd-amdhsa--gfx900"
+// CHECK-SAME: "-targets={{.*}},hipv4-amdgcn-amd-amdhsa--gfx803,hipv4-amdgcn-amd-amdhsa--gfx900"
 // CHECK-SAME: "-inputs={{.*}},[[IMG_DEV1]],[[IMG_DEV2]]" "-outputs=[[BUNDLE:.*hipfb]]"
 
 // CHECK: [[MC:".*llvm-mc.*"]] "-o" [[OBJBUNDLE:".*o"]] "{{.*}}.mcin" "--filetype=obj"
Index: clang/test/Driver/hip-toolchain-rdc-static-lib.hip
===
--- clang/test/Driver/hip-toolchain-rdc-static-lib.hip
+++ clang/test/Driver/hip-toolchain-rdc-static-lib.hip
@@ -85,7 +85,7 @@
 
 // combine images generated into hip fat binary object
 // CHECK: [[BUNDLER:".*clang-offload-bundler"]] "-type=o"
-// CHECK-SAME: "-targets={{.*}},hip-amdgcn-amd-amdhsa--gfx803,hip-amdgcn-amd-amdhsa--gfx900"
+// CHECK-SAME: "-targets={{.*}},hipv4-amdgcn-amd-amdhsa--gfx803,hipv4-amdgcn-amd-amdhsa--gfx900"
 // CHECK-SAME: "-inputs={{.*}},[[IMG_DEV1]],[[IMG_DEV2]]" "-outputs=[[BUNDLE:.*hipfb]]"
 
 // CHECK: [[MC:".*llvm-mc.*"]] "-o" [[OBJBUNDLE:".*o"]] "{{.*}}.mcin" "--filetype=obj"
Index: clang/test/Driver/hip-toolchain-rdc-separate.hip
===
--- clang/test/Driver/hip-toolchain-rdc-separate.hip
+++ clang/test/Driver/hip-toolchain-rdc-separate.hip
@@ -125,7 +125,7 @@
 // LINK-SAME: "-o" "[[IMG_DEV2:.*.out]]" "[[A_BC2]]" "[[B_BC2]]"
 
 // LINK: [[BUNDLER:".*clang-offload-bundler"]] "-type=o"
-// LINK-SAME: "-targets={{.*}},hip-amdgcn-amd-amdhsa--gfx803,hip-amdgcn-amd-amdhsa--gfx900"
+// LINK-SAME: "-targets={{.*}},hipv4-amdgcn-amd-amdhsa--gfx803,hipv4-amdgcn-amd-amdhsa--gfx900"
 // LINK-SAME: "-inputs={{.*}},[[IMG_DEV1]],[[IMG_DEV2]]" "-outputs=[[BUNDLE:.*hipfb]]"
 
 // LINK: {{".*llvm-mc.*"}} "-o" "[[OBJBUNDLE:.*o]]" "{{.*}}.mcin" "--filetype=obj"
Index: clang/test/Driver/hip-toolchain-no-rdc.hip
===
--- clang/test/Driver/hip-toolchain-no-rdc.hip
+++ clang/test/Driver/hip-toolchain-no-rdc.hip
@@ -82,7 +82,7 @@
 
 // CHECK: [[BUNDLER:".*clang-offload-bundler"]] "-type=o"
 // CHECK-SAME: "-bundle-align=4096"
-// CHECK-SAME: "-targets={{.*}},hip-amdgcn-amd-amdhsa--gfx803,hip-amdgcn-amd-amdhsa--gfx900"
+// CHECK-SAME: "-targets={{.*}},hipv4-amdgcn-amd-amdhsa--gfx803,hipv4-amdgcn-amd-amdhsa--gfx900"
 // CHECK-SAME: "-inputs={{.*}},[[IMG_DEV_A_803]],[[IMG_DEV_A_900]]" "-outputs=[[BUNDLE_A:.*hipfb]]"
 
 // CHECK: [[CLANG]] "-cc1" "-triple" "x86_64-unknown-linux-gnu"
@@ -145,7 +145,7 @@
 
 // CHECK: [[BUNDLER:".*clang-offload-bundler"]] "-type=o"
 // CHECK-SAME: "-bundle-align=4096"
-// CHECK-SAME: "-targets={{.*}},hip-amdgcn-amd-amdhsa--gfx803,hip-amdgcn-amd-amdhsa--gfx900"
+// CHECK-SAME: "-targets={{.*}},hipv4-amdgcn-amd-amdhsa--gfx803,hipv4-amdgcn-amd-amdhsa--gfx900"
 // CHECK-SAME: "-inputs={{.*}},[[IMG_DEV_B_803]],[[IMG_DEV_B_900]]" "-outputs=[[BUNDLE_A:.*hipfb]]"
 
 // CHECK: [[CLANG]] "-cc1" "-triple" "x86_64-unknown-linux-gnu"
Index: clang/test/Driver/hip-toolchain-device-only.hip
===
--- 

[clang] 4fd05e0 - [HIP] Change to code object v4

[Yaxun Liu via cfe-commits]

Author: Yaxun (Sam) Liu
Date: 2021-04-06T20:22:58-04:00
New Revision: 4fd05e0ad7fba41f27a6f61d9f7fec4382cb96fe

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

LOG: [HIP] Change to code object v4

Change to code object v4 by default to match ROCm 4.1.

Reviewed by: Artem Belevich

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

Added: 


Modified: 
clang/lib/Driver/ToolChains/CommonArgs.cpp
clang/lib/Driver/ToolChains/HIP.cpp
clang/test/Driver/hip-code-object-version.hip
clang/test/Driver/hip-target-id.hip
clang/test/Driver/hip-toolchain-device-only.hip
clang/test/Driver/hip-toolchain-no-rdc.hip
clang/test/Driver/hip-toolchain-rdc-separate.hip
clang/test/Driver/hip-toolchain-rdc-static-lib.hip
clang/test/Driver/hip-toolchain-rdc.hip
clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp

Removed: 




diff  --git a/clang/lib/Driver/ToolChains/CommonArgs.cpp 
b/clang/lib/Driver/ToolChains/CommonArgs.cpp
index 6baf1d1acbb32..10d61ae9828f8 100644
--- a/clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ b/clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -1577,7 +1577,7 @@ unsigned tools::getOrCheckAMDGPUCodeObjectVersion(
 const Driver , const llvm::opt::ArgList , bool Diagnose) {
   const unsigned MinCodeObjVer = 2;
   const unsigned MaxCodeObjVer = 4;
-  unsigned CodeObjVer = 3;
+  unsigned CodeObjVer = 4;
 
   // Emit warnings for legacy options even if they are overridden.
   if (Diagnose) {

diff  --git a/clang/lib/Driver/ToolChains/HIP.cpp 
b/clang/lib/Driver/ToolChains/HIP.cpp
index b5242689fffd4..c0777bd39d321 100644
--- a/clang/lib/Driver/ToolChains/HIP.cpp
+++ b/clang/lib/Driver/ToolChains/HIP.cpp
@@ -108,11 +108,12 @@ void AMDGCN::constructHIPFatbinCommand(Compilation , 
const JobAction ,
   std::string BundlerTargetArg = "-targets=host-x86_64-unknown-linux";
   std::string BundlerInputArg = "-inputs=" NULL_FILE;
 
-  // TODO: Change the bundle ID as requested by HIP runtime.
   // For code object version 2 and 3, the offload kind in bundle ID is 'hip'
   // for backward compatibility. For code object version 4 and greater, the
   // offload kind in bundle ID is 'hipv4'.
   std::string OffloadKind = "hip";
+  if (getOrCheckAMDGPUCodeObjectVersion(C.getDriver(), Args) >= 4)
+OffloadKind = OffloadKind + "v4";
   for (const auto  : Inputs) {
 const auto* A = II.getAction();
 BundlerTargetArg = BundlerTargetArg + "," + OffloadKind +

diff  --git a/clang/test/Driver/hip-code-object-version.hip 
b/clang/test/Driver/hip-code-object-version.hip
index 6e4e96688593c..ba73ace411b5a 100644
--- a/clang/test/Driver/hip-code-object-version.hip
+++ b/clang/test/Driver/hip-code-object-version.hip
@@ -45,7 +45,7 @@
 // RUN:   %s 2>&1 | FileCheck -check-prefix=V4 %s
 
 // V4: "-mllvm" "--amdhsa-code-object-version=4"
-// V4: "-targets=host-x86_64-unknown-linux,hip-amdgcn-amd-amdhsa--gfx906"
+// V4: "-targets=host-x86_64-unknown-linux,hipv4-amdgcn-amd-amdhsa--gfx906"
 
 // Check bundle ID for code object version default
 
@@ -53,8 +53,8 @@
 // RUN:   --offload-arch=gfx906 -nogpulib \
 // RUN:   %s 2>&1 | FileCheck -check-prefix=VD %s
 
-// VD: "-mllvm" "--amdhsa-code-object-version=3"
-// VD: "-targets=host-x86_64-unknown-linux,hip-amdgcn-amd-amdhsa--gfx906"
+// VD: "-mllvm" "--amdhsa-code-object-version=4"
+// VD: "-targets=host-x86_64-unknown-linux,hipv4-amdgcn-amd-amdhsa--gfx906"
 
 // Check invalid code object version option.
 

diff  --git a/clang/test/Driver/hip-target-id.hip 
b/clang/test/Driver/hip-target-id.hip
index aee44a1dcd1c0..0507cc066a181 100644
--- a/clang/test/Driver/hip-target-id.hip
+++ b/clang/test/Driver/hip-target-id.hip
@@ -47,7 +47,7 @@
 // CHECK-SAME: "-plugin-opt=-mattr=-sramecc,+xnack"
 
 // CHECK: {{"[^"]*clang-offload-bundler[^"]*"}}
-// CHECK-SAME: 
"-targets=host-x86_64-unknown-linux,hip-amdgcn-amd-amdhsa--gfx908:sramecc+:xnack+,hip-amdgcn-amd-amdhsa--gfx908:sramecc-:xnack+"
+// CHECK-SAME: 
"-targets=host-x86_64-unknown-linux,hipv4-amdgcn-amd-amdhsa--gfx908:sramecc+:xnack+,hipv4-amdgcn-amd-amdhsa--gfx908:sramecc-:xnack+"
 
 // Check canonicalization and repeating of target ID.
 
@@ -58,7 +58,7 @@
 // RUN:   --offload-arch=fiji \
 // RUN:   --rocm-path=%S/Inputs/rocm \
 // RUN:   %s 2>&1 | FileCheck -check-prefix=FIJI %s
-// FIJI: "-targets=host-x86_64-unknown-linux,hip-amdgcn-amd-amdhsa--gfx803"
+// FIJI: "-targets=host-x86_64-unknown-linux,hipv4-amdgcn-amd-amdhsa--gfx803"
 
 // RUN: %clang -### -target x86_64-linux-gnu \
 // RUN:   -x hip \
@@ -69,4 +69,4 @@
 // RUN:   --offload-arch=gfx906 \
 // RUN:   --rocm-path=%S/Inputs/rocm \
 // RUN:   %s 2>&1 | FileCheck -check-prefix=MULTI %s
-// MULTI: 

[PATCH] D77574: [OpenMP] Fix layering problem with FrontendOpenMP

[Tom Stellard via Phabricator via cfe-commits]

tstellar added inline comments.
Herald added a reviewer: bollu.
Herald added a subscriber: sstefan1.
Herald added a project: clang-tools-extra.



Comment at: clang/lib/ASTMatchers/CMakeLists.txt:17-18
   )
+
+target_link_libraries(clangASTMatchers PUBLIC LLVMFrontendOpenMP)

My guess is that this is the problem. Making this change will cause clang 
binaries to link against the LLVMFrontendOpenMP static library in addition to 
libLLVM.so (which already contains this library).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77574

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

[PATCH] D99037: [Matrix] Implement C-style explicit type conversions for matrix types

[Saurabh Jha via Phabricator via cfe-commits]

SaurabhJha updated this revision to Diff 335680.
SaurabhJha added a comment.

Fix the bug with int <-> float conversion by explicitly passing llvm types to 
EmitCastBetweenScalarTypes


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99037

Files:
  clang/include/clang/AST/OperationKinds.def
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/Expr.cpp
  clang/lib/AST/ExprConstant.cpp
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/CodeGen/CGExprAgg.cpp
  clang/lib/CodeGen/CGExprComplex.cpp
  clang/lib/CodeGen/CGExprConstant.cpp
  clang/lib/CodeGen/CGExprScalar.cpp
  clang/lib/Edit/RewriteObjCFoundationAPI.cpp
  clang/lib/Sema/SemaCast.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp
  clang/test/CodeGen/matrix-cast.c
  clang/test/Sema/matrix-cast.c
  clang/test/SemaCXX/matrix-casts.cpp

Index: clang/test/SemaCXX/matrix-casts.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/matrix-casts.cpp
@@ -0,0 +1,67 @@
+// RUN: %clang_cc1 -std=c++11 -fenable-matrix -fsyntax-only -verify %s
+
+template 
+
+using matrix_4_4 = X __attribute__((matrix_type(4, 4)));
+
+template 
+
+using matrix_5_5 = Y __attribute__((matrix_type(5, 5)));
+
+typedef struct test_struct {
+} test_struct;
+
+typedef int vec __attribute__((vector_size(4)));
+
+void f1() {
+  // TODO: Update this test once the support of C-style casts for C++ is implemented.
+  matrix_4_4 m1;
+  matrix_4_4 m2;
+  matrix_4_4 m3;
+  matrix_5_5 m4;
+  int i;
+  vec v;
+  test_struct *s;
+
+  (matrix_4_4)m1;   // expected-error {{C-style cast from 'matrix_4_4' (aka 'char __attribute__((matrix_type(4, \
+4)))') to 'matrix_4_4' (aka 'int __attribute__((matrix_type(4, 4)))') is not allowed}}
+  (matrix_4_4)m2; // expected-error {{C-style cast from 'matrix_4_4' (aka 'int __attribute__((matrix_type(4, \
+4)))') to 'matrix_4_4' (aka 'short __attribute__((matrix_type(4, 4)))') is not allowed}}
+  (matrix_5_5)m3;   // expected-error {{C-style cast from 'matrix_4_4' (aka 'short __attribute__((matrix_type(4, \
+4)))') to 'matrix_5_5' (aka 'int __attribute__((matrix_type(5, 5)))') is not allowed}}
+
+  (int)m3;// expected-error {{C-style cast from 'matrix_4_4' (aka 'short __attribute__((matrix_type(4, \
+4)))') to 'int'}}
+  (matrix_4_4)i; // expected-error {{C-style cast from 'int' to 'matrix_4_4' (aka 'int __attribute__((\
+matrix_type(4, 4)))') is not allowed}}
+
+  (vec) m2;// expected-error {{C-style cast from 'matrix_4_4' (aka 'int __attribute__((matrix_type(4, 4)))') \
+to 'vec' (vector of 1 'int' value) is not allowed}}
+  (matrix_4_4)v; // expected-error {{C-style cast from 'vec' (vector of 1 'int' value) to 'matrix_4_4' \
+(aka 'char __attribute__((matrix_type(4, 4)))') is not allowed}}
+
+  (test_struct *)m1;// expected-error {{cannot cast from type 'matrix_4_4' (aka 'char __attribute__\
+((matrix_type(4, 4)))') to pointer type 'test_struct *}}'
+  (matrix_5_5)s; // expected-error {{C-style cast from 'test_struct *' to 'matrix_5_5' (aka 'float __attribute__\
+((matrix_type(5, 5)))') is not allowed}}'
+}
+
+void f2() {
+  // TODO: Update this test once the support of C-style casts for C++ is implemented.
+  matrix_4_4 m1;
+  matrix_5_5 m2;
+  matrix_5_5 m3;
+  matrix_4_4 m4;
+  float f;
+
+  (matrix_4_4)m1;   // expected-error {{C-style cast from 'matrix_4_4' (aka 'float __attribute__\
+((matrix_type(4, 4)))') to 'matrix_4_4' (aka 'double __attribute__((matrix_type(4, 4)))') is not allowed}}
+  (matrix_5_5)m2;// expected-error {{C-style cast from 'matrix_5_5' (aka 'double __attribute__\
+((matrix_type(5, 5)))') to 'matrix_5_5' (aka 'float __attribute__((matrix_type(5, 5)))') is not allowed}}
+  (matrix_5_5)m3; // expected-error {{C-style cast from 'matrix_5_5' (aka 'int __attribute__\
+((matrix_type(5, 5)))') to 'matrix_5_5' (aka 'unsigned int __attribute__((matrix_type(5, 5)))') \
+is not allowed}}
+  (matrix_4_4)m4;  // expected-error {{C-style cast from 'matrix_4_4' (aka 'unsigned int \
+__attribute__((matrix_type(4, 4)))') to 'matrix_4_4' (aka 'int __attribute__((matrix_type(4, 4)))') is not \
+allowed}}
+}
Index: clang/test/Sema/matrix-cast.c
===
--- /dev/null
+++ clang/test/Sema/matrix-cast.c
@@ -0,0 +1,75 @@
+// RUN: %clang_cc1 -fenable-matrix -fsyntax-only %s -verify
+
+typedef char cx4x4 __attribute__((matrix_type(4, 4)));
+typedef int ix4x4 __attribute__((matrix_type(4, 4)));
+typedef short sx4x4 __attribute__((matrix_type(4, 4)));
+typedef int ix5x5 __attribute__((matrix_type(5, 5)));
+typedef float fx5x5 __attribute__((matrix_type(5, 5)));
+typedef int vec __attribute__((vector_size(4)));
+typedef struct test_struct {
+} test_struct;
+
+void f1() {
+  cx4x4 m1;
+  ix4x4 m2;
+  sx4x4 m3;
+  ix5x5 m4;
+  fx5x5 m5;
+  int i;
+  vec 

[PATCH] D99235: [HIP] Change to code object v4

[Artem Belevich via Phabricator via cfe-commits]

tra added a comment.

Still LGTM.




Comment at: clang/test/Driver/hip-code-object-version.hip:24-39
 // Check bundle ID for code object v2.
 
 // RUN: %clang -### -target x86_64-linux-gnu \
 // RUN:   -mno-code-object-v3 \
 // RUN:   --offload-arch=gfx906 -nogpulib \
 // RUN:   %s 2>&1 | FileCheck -check-prefixes=V2,V2-WARN %s
 

Nit: it would be nice to move V2 tests above the V3, so the tests are in order. 


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

https://reviews.llvm.org/D99235

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

[PATCH] D99893: [WIP] Replace std::forward & std::move by cast expressions during Sema

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

rsmith added a comment.

In D99893#2672563 , @cor3ntin wrote:

> This would solve some of the issues (perfect code-gen / no body 
> instantiation), but the declarations are still instantiated of course.
> And I have a few folks asking me whether I could get rid of it. 
> Unfortunately, I think the goals of preserving the AST as is today and 
> getting rid of these template instantiations are contradictory.

Do you know how much of the cost here is instantiating the function 
declaration, and how much is instantiating `std::remove_reference`? I imagine 
we could make the latter cheaper by adding a `__remove_reference` builtin and 
changing libc++'s `forward` and `move` to use it if that's worthwhile.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99893

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

[PATCH] D99235: [HIP] Change to code object v4

[Yaxun Liu via Phabricator via cfe-commits]

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



Comment at: clang/lib/Driver/ToolChains/HIP.cpp:115
   std::string OffloadKind = "hip";
+  if (getOrCheckAMDGPUCodeObjectVersion(C.getDriver(), Args) >= 4)
+OffloadKind = OffloadKind + "v4";

tra wrote:
> Should it be an error if we pass `-mcode-object-version=99` ?
Yes we diagnose that.


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

https://reviews.llvm.org/D99235

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

[PATCH] D89013: [libcxx] Support per-target __config_site in per-target runtime build

[Petr Hosek via Phabricator via cfe-commits]

phosek added a comment.

@ldionne any more thoughts on this?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89013

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

[PATCH] D99791: [CGCall] Annotate pointer argument with alignment

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

In D99791#2672528 , @lebedev.ri wrote:

> @rjmccall thank you for taking a look!
>
> In D99791#2670333 , @rjmccall wrote:
>
>> The last major conversation we had about this was this RFC I sent out about 
>> five years ago:
>>
>>   https://lists.llvm.org/pipermail/llvm-dev/2016-January/094012.html
>>
>> In that RFC, I specifically argued that we should not do this, and that it 
>> should only be considered undefined behavior to actually access memory 
>> through a misaligned pointer (for a particular definition of "access 
>> memory").  At Apple, we have made that a promise to our internal users, so 
>> even if we decide to do this, we will need to not do it on Darwin.  However, 
>> as I remember it, the LLVM community did not reach a consensus to adopt my 
>> recommendation, so in principle we still have the flexibility to start doing 
>> this.  I continue to believe that doing so would be a mistake.  At any rate, 
>> you should start by reading that thread.
>
> Thank you for the pointer!
> I can for sure provide an opt-out, however note that in the end it will cause 
> performance regressions as compared to the current LLVM optimizations.

How so?  We do not currently assume that pointer arguments are aligned, or even 
dereferenceable.

I don't mind making stronger assumptions about the C++ `this` argument, but we 
really should not do this for arbitrary pointer arguments.  If you want to put 
together a UBSan mode that checks this, that's fine, but I do not think there 
is a path forward for actually optimizing based on this assumption by default.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99791

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

[PATCH] D98799: [UniqueLinkageName] Use consistent checks when mangling symbo linkage name and debug linkage name.

[David Blaikie via Phabricator via cfe-commits]

dblaikie added inline comments.



Comment at: clang/test/CodeGen/unique-internal-linkage-names-dwarf.c:34-39
+static int go(a) int a;
+{
+  return glob + a;
+}
+
+

hoy wrote:
> dblaikie wrote:
> > hoy wrote:
> > > dblaikie wrote:
> > > > hoy wrote:
> > > > > dblaikie wrote:
> > > > > > hoy wrote:
> > > > > > > dblaikie wrote:
> > > > > > > > hoy wrote:
> > > > > > > > > dblaikie wrote:
> > > > > > > > > > hoy wrote:
> > > > > > > > > > > dblaikie wrote:
> > > > > > > > > > > > Does this need to be down here? Or would the code be a 
> > > > > > > > > > > > well exercised if it was up next to the go declaration 
> > > > > > > > > > > > above?
> > > > > > > > > > > Yes, it needs to be here. Otherwise it will just like the 
> > > > > > > > > > > function `bar` above that doesn't get a uniquefied name. 
> > > > > > > > > > > I think moving the definition up to right after the 
> > > > > > > > > > > declaration hides the declaration.
> > > > > > > > > > Not sure I follow - do you mean that if the go declaration 
> > > > > > > > > > and go definition were next to each other, this test would 
> > > > > > > > > > (mechanically speaking) not validate what the patch? Or 
> > > > > > > > > > that it would be less legible, but still mechanically 
> > > > > > > > > > correct?
> > > > > > > > > > 
> > > > > > > > > > I think it would be (assuming it's still mechanically 
> > > > > > > > > > correct) more legible to put the declaration next to the 
> > > > > > > > > > definition - the comment describes why the declaration is 
> > > > > > > > > > significant/why the definition is weird, and seeing all 
> > > > > > > > > > that together would be clearer to me than spreading it 
> > > > > > > > > > out/having to look further away to see what's going on.
> > > > > > > > > When the `go` declaration and `go` definition were next to 
> > > > > > > > > each other, the go function won't get a uniqufied name at 
> > > > > > > > > all. The declaration will be overwritten by the definition. 
> > > > > > > > > Only when the declaration is seen by others, such the 
> > > > > > > > > callsite in `baz`, the declaration makes a difference by 
> > > > > > > > > having the callsite use a uniqufied name.
> > > > > > > > > 
> > > > > > > > > 
> > > > > > > > Ah! Interesting, good to know. 
> > > > > > > > 
> > > > > > > > Is that worth supporting, I wonder? I guess it falls out for 
> > > > > > > > free/without significant additional complexity. I worry about 
> > > > > > > > the subtlety of the additional declaration changing the 
> > > > > > > > behavior here... might be a bit surprising/subtle. But maybe no 
> > > > > > > > nice way to avoid it either.
> > > > > > > It would be ideal if user never writes code like that. 
> > > > > > > Unfortunately it exists with legacy code (such as mysql). I think 
> > > > > > > it's worth supporting it from AutoFDO point of view to avoid a 
> > > > > > > silent mismatch between debug linkage name and real linkage name.
> > > > > > Oh, I agree that we shouldn't mismatch debug info and the actual 
> > > > > > symbol name - what I meant was whether code like this should get 
> > > > > > mangled or not when using unique-internal-linkage names.
> > > > > > 
> > > > > > I'm now more curious about this:
> > > > > > 
> > > > > > > When the `go` declaration and `go` definition were next to each 
> > > > > > > other, the go function won't get a uniqufied name at all.
> > > > > > 
> > > > > > This doesn't seem to happen with the 
> > > > > > `__attribute__((overloadable))` attribute, for instance - so any 
> > > > > > idea what's different about uniquification that's working 
> > > > > > differently than overloadable?
> > > > > > 
> > > > > > ```
> > > > > > $ cat test.c
> > > > > > __attribute__((overloadable)) static int go(a) int a; {
> > > > > >   return 3 + a;
> > > > > > }
> > > > > > void baz() {
> > > > > >   go(2);
> > > > > > }
> > > > > > $ clang-tot test.c -emit-llvm -S -o - | grep go
> > > > > >   %call = call i32 @_ZL2goi(i32 2)
> > > > > > define internal i32 @_ZL2goi(i32 %a) #0 {
> > > > > > ```
> > > > > Good question. I'm not sure what's exactly going on but it looks like 
> > > > > with the overloadable attribute, the old-style definition is treated 
> > > > > as having prototype. But if you do this:
> > > > > 
> > > > > ```
> > > > > __attribute__((overloadable)) 
> > > > > void baz() {}
> > > > > ```
> > > > > then there's the error:
> > > > > 
> > > > > ```
> > > > > error: 'overloadable' function 'baz' must have a prototype
> > > > > void baz() {
> > > > > ```
> > > > > 
> > > > > `void baz() {` does not come with a prototype. That's for sure.  
> > > > > Sounds like `int go(a) int a {;` can have a prototype when it is 
> > > > > loadable. I'm wondering why it's not always treated as having 
> > > > > prototype, since the parameter type is there.
> > > > > 
> > > > > 
> > > > > 
> > > > Yeah, that seems like that divergence be worth understanding (& if 
> > 

[PATCH] D99037: [Matrix] Implement C-style explicit type conversions for matrix types

[Saurabh Jha via Phabricator via cfe-commits]

SaurabhJha updated this revision to Diff 335677.
SaurabhJha added a comment.

I reverted the int <-> float conversion to previous code to make the tests 
pass. That way, we atleast have something working and we can go from there.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99037

Files:
  clang/include/clang/AST/OperationKinds.def
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/Expr.cpp
  clang/lib/AST/ExprConstant.cpp
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/CodeGen/CGExprAgg.cpp
  clang/lib/CodeGen/CGExprComplex.cpp
  clang/lib/CodeGen/CGExprConstant.cpp
  clang/lib/CodeGen/CGExprScalar.cpp
  clang/lib/Edit/RewriteObjCFoundationAPI.cpp
  clang/lib/Sema/SemaCast.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp
  clang/test/CodeGen/matrix-cast.c
  clang/test/Sema/matrix-cast.c
  clang/test/SemaCXX/matrix-casts.cpp

Index: clang/test/SemaCXX/matrix-casts.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/matrix-casts.cpp
@@ -0,0 +1,67 @@
+// RUN: %clang_cc1 -std=c++11 -fenable-matrix -fsyntax-only -verify %s
+
+template 
+
+using matrix_4_4 = X __attribute__((matrix_type(4, 4)));
+
+template 
+
+using matrix_5_5 = Y __attribute__((matrix_type(5, 5)));
+
+typedef struct test_struct {
+} test_struct;
+
+typedef int vec __attribute__((vector_size(4)));
+
+void f1() {
+  // TODO: Update this test once the support of C-style casts for C++ is implemented.
+  matrix_4_4 m1;
+  matrix_4_4 m2;
+  matrix_4_4 m3;
+  matrix_5_5 m4;
+  int i;
+  vec v;
+  test_struct *s;
+
+  (matrix_4_4)m1; // expected-error {{C-style cast from 'matrix_4_4' (aka 'char __attribute__((matrix_type(4, \
+4)))') to 'matrix_4_4' (aka 'int __attribute__((matrix_type(4, 4)))') is not allowed}}
+  (matrix_4_4)m2; // expected-error {{C-style cast from 'matrix_4_4' (aka 'int __attribute__((matrix_type(4, \
+4)))') to 'matrix_4_4' (aka 'short __attribute__((matrix_type(4, 4)))') is not allowed}}
+  (matrix_5_5)m3; // expected-error {{C-style cast from 'matrix_4_4' (aka 'short __attribute__((matrix_type(4, \
+4)))') to 'matrix_5_5' (aka 'int __attribute__((matrix_type(5, 5)))') is not allowed}}
+
+  (int)m3; // expected-error {{C-style cast from 'matrix_4_4' (aka 'short __attribute__((matrix_type(4, \
+4)))') to 'int'}}
+  (matrix_4_4)i; // expected-error {{C-style cast from 'int' to 'matrix_4_4' (aka 'int __attribute__((\
+matrix_type(4, 4)))') is not allowed}}
+
+  (vec)m2; // expected-error {{C-style cast from 'matrix_4_4' (aka 'int __attribute__((matrix_type(4, 4)))') \
+to 'vec' (vector of 1 'int' value) is not allowed}}
+  (matrix_4_4)v; // expected-error {{C-style cast from 'vec' (vector of 1 'int' value) to 'matrix_4_4' \
+(aka 'char __attribute__((matrix_type(4, 4)))') is not allowed}}
+
+  (test_struct *)m1; // expected-error {{cannot cast from type 'matrix_4_4' (aka 'char __attribute__\
+((matrix_type(4, 4)))') to pointer type 'test_struct *}}'
+  (matrix_5_5)s; // expected-error {{C-style cast from 'test_struct *' to 'matrix_5_5' (aka 'float __attribute__\
+((matrix_type(5, 5)))') is not allowed}}'
+}
+
+void f2() {
+  // TODO: Update this test once the support of C-style casts for C++ is implemented.
+  matrix_4_4 m1;
+  matrix_5_5 m2;
+  matrix_5_5 m3;
+  matrix_4_4 m4;
+  float f;
+
+  (matrix_4_4)m1; // expected-error {{C-style cast from 'matrix_4_4' (aka 'float __attribute__\
+((matrix_type(4, 4)))') to 'matrix_4_4' (aka 'double __attribute__((matrix_type(4, 4)))') is not allowed}}
+  (matrix_5_5)m2; // expected-error {{C-style cast from 'matrix_5_5' (aka 'double __attribute__\
+((matrix_type(5, 5)))') to 'matrix_5_5' (aka 'float __attribute__((matrix_type(5, 5)))') is not allowed}}
+  (matrix_5_5)m3; // expected-error {{C-style cast from 'matrix_5_5' (aka 'int __attribute__\
+((matrix_type(5, 5)))') to 'matrix_5_5' (aka 'unsigned int __attribute__((matrix_type(5, 5)))') \
+is not allowed}}
+  (matrix_4_4)m4; // expected-error {{C-style cast from 'matrix_4_4' (aka 'unsigned int \
+__attribute__((matrix_type(4, 4)))') to 'matrix_4_4' (aka 'int __attribute__((matrix_type(4, 4)))') is not \
+allowed}}
+}
Index: clang/test/Sema/matrix-cast.c
===
--- /dev/null
+++ clang/test/Sema/matrix-cast.c
@@ -0,0 +1,75 @@
+// RUN: %clang_cc1 -fenable-matrix -fsyntax-only %s -verify
+
+typedef char cx4x4 __attribute__((matrix_type(4, 4)));
+typedef int ix4x4 __attribute__((matrix_type(4, 4)));
+typedef short sx4x4 __attribute__((matrix_type(4, 4)));
+typedef int ix5x5 __attribute__((matrix_type(5, 5)));
+typedef float fx5x5 __attribute__((matrix_type(5, 5)));
+typedef int vec __attribute__((vector_size(4)));
+typedef struct test_struct {
+} test_struct;
+
+void f1() {
+  cx4x4 m1;
+  ix4x4 m2;
+  sx4x4 m3;
+  ix5x5 m4;
+  fx5x5 m5;
+  int i;
+  vec v;
+  

[PATCH] D99993: [clang-tidy] bugprone-argument-comment: ignore name mismatches for decls from system headers

[Nathan James via Phabricator via cfe-commits]

njames93 added a comment.

In D3#2672624 , @Eugene.Zelenko 
wrote:

> Why `--header-filter` command line option or `HeaderFilterRegex` 
> configuration file option could not solve this problem?

That's for suppressing warnings emitted in header files. The issue this is 
solving is warnings being emitted at call sites where the callee is in system 
header files.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D3

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

[PATCH] D99683: [HIP] Support ThinLTO

[Teresa Johnson via Phabricator via cfe-commits]

tejohnson added a comment.

In D99683#2672578 , @yaxunl wrote:

> In D99683#2672554 , @tejohnson wrote:
>
>> This raises some higher level questions for me:
>>
>> First, how will you deal with other corner cases that won't or cannot be 
>> imported right now? While enabling importing of noinline functions and 
>> cranking up the threshold will get the majority of functions imported, there 
>> are cases that we still won't import (functions/vars that are interposable, 
>> certain funcs/vars that cannot be renamed, most non-const variables with 
>> non-trivial initializers).
>
> We will document the limitation of thinLTO support of HIP toolchain and 
> recommend users not to use thinLTO in those corner cases.
>
>> Second, force importing of everything transitively referenced defeats the 
>> purpose of ThinLTO and would probably make it worse than regular LTO. The 
>> main entry module will need to import everything transitively referenced 
>> from there, so everything not dead in the binary, which should make that 
>> module post importing equivalent to a regular LTO module. In addition, every 
>> other module needs to transitively import everything referenced from those 
>> modules, making them very large depending on how many leaf vs non-leaf 
>> functions and variables they contain. What is the goal of doing ThinLTO in 
>> this case?
>
> The objective is to improve optimization/codegen time by using multi-threads 
> of thinLTO. For example, I have 10 modules each containing a kernel. In full 
> LTO linking, I get one big module containing 10 kernels with all functions 
> inlined, and I have one thread for optimization/codegen. With thinLTO, I get 
> one kernel in each module, with all functions inlined. AMDGPU internalization 
> and global DCE will remove functions not used by that kernel in each module. 
> I will get 10 threads, each doing optimization/codegen for one kernel. 
> Theoretically, there could be 10 times speed up.

That will work as long as there are no dependence edges anywhere between the 
kernels. Is this a library that has a bunch of totally independent kernels only 
called externally?


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

https://reviews.llvm.org/D99683

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

[PATCH] D99993: [clang-tidy] bugprone-argument-comment: ignore name mismatches for decls from system headers

[George Burgess IV via Phabricator via cfe-commits]

george.burgess.iv added a comment.

Thanks for the comment!

> Why `--header-filter` command line option or `HeaderFilterRegex` 
> configuration file option could not solve this problem?

AFAICT, `--header-filter` & `HeaderFilterRegex` exist to filter diagnostics 
that occur in matching headers. These diagnostics are influenced by the 
contents of system headers, but are ultimately emitted inside of source files 
that have interesting lints, e.g.,

  [ub] /l [ 19ms ] ~> cat test.cpp
  #include 
  void foo(void *a, const void *b, size_t n) {
memcpy(/*blah=*/a, b, n);
  }
  [ub] /l [ 2ms ] ~> clang-tidy --checks='bugprone-argument-comment' test.cpp 
--header-filter='dont-match-anything' --
  1 warning generated.
  /l/test.cpp:3:10: warning: argument name 'blah' in comment does not match 
parameter name '__dest' [bugprone-argument-comment]
memcpy(/*blah=*/a, b, n);
   ^
  /usr/include/string.h:43:39: note: '__dest' declared here
  extern void *memcpy (void *__restrict __dest, const void *__restrict __src,
^
  [ub] /l [ 25ms ] ~> clang-tidy --checks='bugprone-argument-comment' test.cpp 
--config 'HeaderFilterRegex: "dont-match-anything"' --
  1 warning generated.
  /l/test.cpp:3:10: warning: argument name 'blah' in comment does not match 
parameter name '__dest' [bugprone-argument-comment]
memcpy(/*blah=*/a, b, n);
   ^
  /usr/include/string.h:43:39: note: '__dest' declared here
  extern void *memcpy (void *__restrict __dest, const void *__restrict __src,
^
  [ub] /l [ 22ms ] ~> 

So I don't think those help here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D3

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

[PATCH] D99996: [Driver] Drop $DEFAULT_TRIPLE-$name as a fallback program name

[Fangrui Song via Phabricator via cfe-commits]

MaskRay created this revision.
MaskRay added reviewers: atanasyan, doko, nathanchance, nickdesaulniers, 
sylvestre.ledru, vkalintiris.
Herald added subscribers: arichardson, sdardis.
MaskRay requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

D13340  introduced this behavior which is not 
needed even for mips.
This was raised on https://lists.llvm.org/pipermail/cfe-dev/2020-May/065437.html
but no action was taken.

This was raised again in a thread "The LLVM host/target TRIPLE padding drama on 
Debian"
as it caused confusion. This patch drops the behavior.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D6

Files:
  clang/lib/Driver/Driver.cpp
  clang/test/Driver/program-path-priority.c


Index: clang/test/Driver/program-path-priority.c
===
--- clang/test/Driver/program-path-priority.c
+++ clang/test/Driver/program-path-priority.c
@@ -3,8 +3,7 @@
 
 /// Check the priority used when searching for tools
 /// Names and locations are usually in this order:
-/// -tool, tool, -tool
-/// program path, PATH
+/// -tool, tool, program path, PATH
 /// (from highest to lowest priority)
 /// A higher priority name found in a lower priority
 /// location will win over a lower priority name in a
@@ -102,13 +101,11 @@
 // DEFAULT_TRIPLE_NO_NOTREAL: env/gcc"
 // DEFAULT_TRIPLE_NO_NOTREAL-NOT: -gcc"
 
-/// default triple only chosen when no others are present
+/// Pick "gcc" as a fallback. Don't pick $DEFAULT_TRIPLE-gcc.
 // RUN: rm %t/env/gcc
 // RUN: env "PATH=%t/env/" %t/clang -### -target notreal-none-elf %s 2>&1 | \
 // RUN:   FileCheck --check-prefix=DEFAULT_TRIPLE_NO_OTHERS %s
-// DEFAULT_TRIPLE_NO_OTHERS: -gcc"
-// DEFAULT_TRIPLE_NO_OTHERS-NOT: notreal-none-elf-gcc"
-// DEFAULT_TRIPLE_NO_OTHERS-NOT: /gcc"
+// DEFAULT_TRIPLE_NO_OTHERS: "gcc"
 
 /// -B paths are searched separately so default triple will win
 /// if put in one of those even if other paths have higher priority names
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -5070,11 +5070,6 @@
   // FIXME: Needs a better variable than TargetTriple
   Names.emplace_back((TargetTriple + "-" + Tool).str());
   Names.emplace_back(Tool);
-
-  // Allow the discovery of tools prefixed with LLVM's default target triple.
-  std::string DefaultTargetTriple = llvm::sys::getDefaultTargetTriple();
-  if (DefaultTargetTriple != TargetTriple)
-Names.emplace_back((DefaultTargetTriple + "-" + Tool).str());
 }
 
 static bool ScanDirForExecutable(SmallString<128> , StringRef Name) {


Index: clang/test/Driver/program-path-priority.c
===
--- clang/test/Driver/program-path-priority.c
+++ clang/test/Driver/program-path-priority.c
@@ -3,8 +3,7 @@
 
 /// Check the priority used when searching for tools
 /// Names and locations are usually in this order:
-/// -tool, tool, -tool
-/// program path, PATH
+/// -tool, tool, program path, PATH
 /// (from highest to lowest priority)
 /// A higher priority name found in a lower priority
 /// location will win over a lower priority name in a
@@ -102,13 +101,11 @@
 // DEFAULT_TRIPLE_NO_NOTREAL: env/gcc"
 // DEFAULT_TRIPLE_NO_NOTREAL-NOT: -gcc"
 
-/// default triple only chosen when no others are present
+/// Pick "gcc" as a fallback. Don't pick $DEFAULT_TRIPLE-gcc.
 // RUN: rm %t/env/gcc
 // RUN: env "PATH=%t/env/" %t/clang -### -target notreal-none-elf %s 2>&1 | \
 // RUN:   FileCheck --check-prefix=DEFAULT_TRIPLE_NO_OTHERS %s
-// DEFAULT_TRIPLE_NO_OTHERS: -gcc"
-// DEFAULT_TRIPLE_NO_OTHERS-NOT: notreal-none-elf-gcc"
-// DEFAULT_TRIPLE_NO_OTHERS-NOT: /gcc"
+// DEFAULT_TRIPLE_NO_OTHERS: "gcc"
 
 /// -B paths are searched separately so default triple will win
 /// if put in one of those even if other paths have higher priority names
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -5070,11 +5070,6 @@
   // FIXME: Needs a better variable than TargetTriple
   Names.emplace_back((TargetTriple + "-" + Tool).str());
   Names.emplace_back(Tool);
-
-  // Allow the discovery of tools prefixed with LLVM's default target triple.
-  std::string DefaultTargetTriple = llvm::sys::getDefaultTargetTriple();
-  if (DefaultTargetTriple != TargetTriple)
-Names.emplace_back((DefaultTargetTriple + "-" + Tool).str());
 }
 
 static bool ScanDirForExecutable(SmallString<128> , StringRef Name) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D99993: [clang-tidy] bugprone-argument-comment: ignore name mismatches for decls from system headers

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added a comment.

Why `--header-filter` command line option or `HeaderFilterRegex` configuration 
file option could not solve this problem?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D3

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

[PATCH] D99994: [CodeView] Add CodeView support for PGO debug information

[Michael Holman via Phabricator via cfe-commits]

Holman created this revision.
Herald added subscribers: dexonsmith, wenlei, hiraditya.
Holman requested review of this revision.
Herald added projects: clang, LLVM.
Herald added subscribers: llvm-commits, cfe-commits.

This change adds debug information about whether PGO is being used or not.

Microsoft performance tooling (e.g. xperf, WPA) uses this information to show 
whether functions are optimized with PGO or not, as well as whether PGO 
information is invalid.

This information is useful for validating whether training scenarios are 
providing good coverage of real world scenarios, showing if profile data is out 
of date, etc.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D4

Files:
  clang/lib/CodeGen/CGDebugInfo.cpp
  llvm/include/llvm/IR/DIBuilder.h
  llvm/include/llvm/IR/DebugInfoMetadata.h
  llvm/lib/AsmParser/LLParser.cpp
  llvm/lib/Bitcode/Reader/MetadataLoader.cpp
  llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
  llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp
  llvm/lib/IR/DIBuilder.cpp
  llvm/lib/IR/DebugInfo.cpp
  llvm/lib/IR/DebugInfoMetadata.cpp
  llvm/unittests/IR/MetadataTest.cpp

Index: llvm/unittests/IR/MetadataTest.cpp
===
--- llvm/unittests/IR/MetadataTest.cpp
+++ llvm/unittests/IR/MetadataTest.cpp
@@ -97,7 +97,7 @@
 Context, 1, getFile(), "clang", false, "-g", 2, "",
 DICompileUnit::FullDebug, getTuple(), getTuple(), getTuple(),
 getTuple(), getTuple(), 0, true, false,
-DICompileUnit::DebugNameTableKind::Default, false, "/", "");
+DICompileUnit::DebugNameTableKind::Default, false, "/", "", false);
   }
   DIType *getBasicType(StringRef Name) {
 return DIBasicType::get(Context, dwarf::DW_TAG_unspecified_type, Name);
@@ -2110,11 +2110,13 @@
   MDTuple *Macros = getTuple();
   StringRef SysRoot = "/";
   StringRef SDK = "MacOSX.sdk";
+  bool HasPGO = false;
   auto *N = DICompileUnit::getDistinct(
   Context, SourceLanguage, File, Producer, IsOptimized, Flags,
   RuntimeVersion, SplitDebugFilename, EmissionKind, EnumTypes,
   RetainedTypes, GlobalVariables, ImportedEntities, Macros, DWOId, true,
-  false, DICompileUnit::DebugNameTableKind::Default, false, SysRoot, SDK);
+  false, DICompileUnit::DebugNameTableKind::Default, false, SysRoot, SDK,
+  HasPGO);
 
   EXPECT_EQ(dwarf::DW_TAG_compile_unit, N->getTag());
   EXPECT_EQ(SourceLanguage, N->getSourceLanguage());
@@ -2133,6 +2135,7 @@
   EXPECT_EQ(DWOId, N->getDWOId());
   EXPECT_EQ(SysRoot, N->getSysRoot());
   EXPECT_EQ(SDK, N->getSDK());
+  EXPECT_EQ(HasPGO, N->hasPGO());
 
   TempDICompileUnit Temp = N->clone();
   EXPECT_EQ(dwarf::DW_TAG_compile_unit, Temp->getTag());
@@ -2151,6 +2154,7 @@
   EXPECT_EQ(Macros, Temp->getMacros().get());
   EXPECT_EQ(SysRoot, Temp->getSysRoot());
   EXPECT_EQ(SDK, Temp->getSDK());
+  EXPECT_EQ(HasPGO, Temp->hasPGO());
 
   auto *TempAddress = Temp.get();
   auto *Clone = MDNode::replaceWithPermanent(std::move(Temp));
@@ -2173,11 +2177,12 @@
   uint64_t DWOId = 0xc0ffee;
   StringRef SysRoot = "/";
   StringRef SDK = "MacOSX.sdk";
+  bool HasPGO = false;
   auto *N = DICompileUnit::getDistinct(
   Context, SourceLanguage, File, Producer, IsOptimized, Flags,
   RuntimeVersion, SplitDebugFilename, EmissionKind, EnumTypes,
   RetainedTypes, nullptr, ImportedEntities, nullptr, DWOId, true, false,
-  DICompileUnit::DebugNameTableKind::Default, false, SysRoot, SDK);
+  DICompileUnit::DebugNameTableKind::Default, false, SysRoot, SDK, HasPGO);
 
   auto *GlobalVariables = MDTuple::getDistinct(Context, None);
   EXPECT_EQ(nullptr, N->getGlobalVariables().get());
Index: llvm/lib/IR/DebugInfoMetadata.cpp
===
--- llvm/lib/IR/DebugInfoMetadata.cpp
+++ llvm/lib/IR/DebugInfoMetadata.cpp
@@ -736,7 +736,7 @@
 Metadata *GlobalVariables, Metadata *ImportedEntities, Metadata *Macros,
 uint64_t DWOId, bool SplitDebugInlining, bool DebugInfoForProfiling,
 unsigned NameTableKind, bool RangesBaseAddress, MDString *SysRoot,
-MDString *SDK, StorageType Storage, bool ShouldCreate) {
+MDString *SDK, bool HasPGO, StorageType Storage, bool ShouldCreate) {
   assert(Storage != Uniqued && "Cannot unique DICompileUnit");
   assert(isCanonical(Producer) && "Expected canonical MDString");
   assert(isCanonical(Flags) && "Expected canonical MDString");
@@ -757,7 +757,7 @@
Context, Storage, SourceLanguage, IsOptimized,
RuntimeVersion, EmissionKind, DWOId, SplitDebugInlining,
DebugInfoForProfiling, NameTableKind, RangesBaseAddress,
-   Ops),
+   HasPGO, Ops),
Storage);
 }
 
Index: llvm/lib/IR/DebugInfo.cpp
===
--- llvm/lib/IR/DebugInfo.cpp
+++ llvm/lib/IR/DebugInfo.cpp
@@ -491,7 +491,8 @@

[PATCH] D99235: [HIP] Change to code object v4

[Artem Belevich via Phabricator via cfe-commits]

tra accepted this revision.
tra added inline comments.
This revision is now accepted and ready to land.



Comment at: clang/lib/Driver/ToolChains/HIP.cpp:115
   std::string OffloadKind = "hip";
+  if (getOrCheckAMDGPUCodeObjectVersion(C.getDriver(), Args) >= 4)
+OffloadKind = OffloadKind + "v4";

Should it be an error if we pass `-mcode-object-version=99` ?


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

https://reviews.llvm.org/D99235

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

[PATCH] D99893: [WIP] Replace std::forward & std::move by cast expressions during Sema

[Arthur O'Dwyer via Phabricator via cfe-commits]

Quuxplusone added a comment.

No matter how it works internally, I think that (nearer the end of the process) 
someone should insist that you add some Clang tests to verify e.g.

  Widget&& a = static_cast(Widget());  // is lifetime-extended, but
  Widget&& b = std::move(Widget());  // is not

https://godbolt.org/z/9Ka7hcode
and whatever other corner-cases people can think of. That is, these may turn 
into not-a-function-calls internally, but they'd better continue to behave 
//observably// as if they were function-calls.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99893

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

[PATCH] D99447: [OpenMP] Define omp_is_initial_device() variants in omp.h

[Hansang Bae via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG3da61ddae7fe: [OpenMP] Define omp_is_initial_device() 
variants in omp.h (authored by hbae).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99447

Files:
  clang/include/clang/Basic/Builtins.def
  clang/lib/AST/ExprConstant.cpp
  clang/test/OpenMP/is_initial_device.c
  openmp/libomptarget/test/api/is_initial_device.c
  openmp/runtime/src/include/omp.h.var

Index: openmp/runtime/src/include/omp.h.var
===
--- openmp/runtime/src/include/omp.h.var
+++ openmp/runtime/src/include/omp.h.var
@@ -468,6 +468,15 @@
 /* OpenMP 5.1 Display Environment */
 extern void omp_display_env(int verbose);
 
+#   if defined(_OPENMP) && _OPENMP >= 201811
+#pragma omp begin declare variant match(device={kind(host)})
+static inline int omp_is_initial_device(void) { return 1; }
+#pragma omp end declare variant
+#pragma omp begin declare variant match(device={kind(nohost)})
+static inline int omp_is_initial_device(void) { return 0; }
+#pragma omp end declare variant
+#   endif
+
 #   undef __KAI_KMPC_CONVENTION
 #   undef __KMP_IMP
 
Index: openmp/libomptarget/test/api/is_initial_device.c
===
--- /dev/null
+++ openmp/libomptarget/test/api/is_initial_device.c
@@ -0,0 +1,30 @@
+// RUN: %libomptarget-compile-run-and-check-x86_64-pc-linux-gnu
+// RUN: %libomptarget-compile-x86_64-pc-linux-gnu -DUNUSED -Wall -Werror
+
+#include 
+#include 
+
+int main() {
+  int errors = 0;
+#ifdef UNUSED
+// Test if it is OK to leave the variants unused in the header
+#else // UNUSED
+  int host = omp_is_initial_device();
+  int device = 1;
+#pragma omp target map(tofrom : device)
+  { device = omp_is_initial_device(); }
+  if (!host) {
+printf("omp_is_initial_device() returned false on host\n");
+errors++;
+  }
+  if (device) {
+printf("omp_is_initial_device() returned true on device\n");
+errors++;
+  }
+#endif // UNUSED
+
+  // CHECK: PASS
+  printf("%s\n", errors ? "FAIL" : "PASS");
+
+  return errors;
+}
Index: clang/test/OpenMP/is_initial_device.c
===
--- clang/test/OpenMP/is_initial_device.c
+++ /dev/null
@@ -1,41 +0,0 @@
-// REQUIRES: powerpc-registered-target
-
-// RUN: %clang_cc1 -verify -fopenmp -x c -triple powerpc64le-unknown-unknown -fopenmp-targets=powerpc64le-unknown-unknown \
-// RUN:-emit-llvm-bc %s -o %t-ppc-host.bc
-// RUN: %clang_cc1 -verify -fopenmp -x ir -triple powerpc64le-unknown-unknown -emit-llvm \
-// RUN: %t-ppc-host.bc -o - | FileCheck %s -check-prefixes HOST,OUTLINED
-// RUN: %clang_cc1 -verify -fopenmp -x c -triple powerpc64le-unknown-unknown -emit-llvm -fopenmp-is-device \
-// RUN: %s -fopenmp-host-ir-file-path %t-ppc-host.bc -o - | FileCheck %s -check-prefixes DEVICE,OUTLINED
-
-// RUN: %clang_cc1 -verify -fopenmp-simd -x c -triple powerpc64le-unknown-unknown -fopenmp-targets=powerpc64le-unknown-unknown -emit-llvm-bc %s -o %t-ppc-host.bc
-// RUN: %clang_cc1 -verify -fopenmp-simd -x ir -triple powerpc64le-unknown-unknown -emit-llvm %t-ppc-host.bc -o - | FileCheck --check-prefix SIMD-ONLY0 %s
-// RUN: %clang_cc1 -verify -fopenmp-simd -x c -triple powerpc64le-unknown-unknown -emit-llvm -fopenmp-is-device %s -fopenmp-host-ir-file-path %t-ppc-host.bc -o - | FileCheck --check-prefix SIMD-ONLY0 %s
-// SIMD-ONLY0-NOT: {{__kmpc|__tgt}}
-
-// expected-no-diagnostics
-int check() {
-  int host = omp_is_initial_device();
-  int device;
-#pragma omp target map(tofrom: device)
-  {
-device = omp_is_initial_device();
-  }
-
-  return host + device;
-}
-
-// The host should get a value of 1:
-// HOST: define{{.*}} @check()
-// HOST: [[HOST:%.*]] = alloca i32
-// HOST: store i32 1, i32* [[HOST]]
-
-// OUTLINED: define{{.*}} @{{.*}}omp_offloading{{.*}}(i32*{{.*}} [[DEVICE_ARGUMENT:%.*]])
-// OUTLINED: [[DEVICE_ADDR_STORAGE:%.*]] = alloca i32*
-// OUTLINED: store i32* [[DEVICE_ARGUMENT]], i32** [[DEVICE_ADDR_STORAGE]]
-// OUTLINED: [[DEVICE_ADDR:%.*]] = load i32*, i32** [[DEVICE_ADDR_STORAGE]]
-
-// The outlined function that is called as fallback also runs on the host:
-// HOST: store i32 1, i32* [[DEVICE_ADDR]]
-
-// The device should get a value of 0:
-// DEVICE: store i32 0, i32* [[DEVICE_ADDR]]
Index: clang/lib/AST/ExprConstant.cpp
===
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -12010,9 +12010,6 @@
 return BuiltinOp == Builtin::BI__atomic_always_lock_free ?
 Success(0, E) : Error(E);
   }
-  case Builtin::BIomp_is_initial_device:
-// We can decide statically which value the runtime would return if called.
-return Success(Info.getLangOpts().OpenMPIsDevice ? 0 : 1, E);
   

[clang] 3da61dd - [OpenMP] Define omp_is_initial_device() variants in omp.h

[Hansang Bae via cfe-commits]

Author: Hansang Bae
Date: 2021-04-06T16:58:01-05:00
New Revision: 3da61ddae7fe77f71b89ce20cf6b5febd68d216a

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

LOG: [OpenMP] Define omp_is_initial_device() variants in omp.h

omp_is_initial_device() is marked as a built-in function in the current
compiler, and user code guarded by this call may be optimized away,
resulting in undesired behavior in some cases. This patch provides a
possible fix for such cases by defining the routine as a variant
function and removing it from builtin list.

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

Added: 
openmp/libomptarget/test/api/is_initial_device.c

Modified: 
clang/include/clang/Basic/Builtins.def
clang/lib/AST/ExprConstant.cpp
openmp/runtime/src/include/omp.h.var

Removed: 
clang/test/OpenMP/is_initial_device.c



diff  --git a/clang/include/clang/Basic/Builtins.def 
b/clang/include/clang/Basic/Builtins.def
index 153e22f00f522..8518f3789f51a 100644
--- a/clang/include/clang/Basic/Builtins.def
+++ b/clang/include/clang/Basic/Builtins.def
@@ -1636,9 +1636,6 @@ LANGBUILTIN(__builtin_load_halff, "fhC*", "nc", 
ALL_OCLC_LANGUAGES)
 BUILTIN(__builtin_os_log_format_buffer_size, "zcC*.", "p:0:nut")
 BUILTIN(__builtin_os_log_format, "v*v*cC*.", "p:0:nt")
 
-// OpenMP 4.0
-LANGBUILTIN(omp_is_initial_device, "i", "nc", OMP_LANG)
-
 // CUDA/HIP
 LANGBUILTIN(__builtin_get_device_side_mangled_name, "cC*.", "ncT", CUDA_LANG)
 

diff  --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp
index b42f3b695ec57..fe6573b8bd2c1 100644
--- a/clang/lib/AST/ExprConstant.cpp
+++ b/clang/lib/AST/ExprConstant.cpp
@@ -12010,9 +12010,6 @@ bool IntExprEvaluator::VisitBuiltinCallExpr(const 
CallExpr *E,
 return BuiltinOp == Builtin::BI__atomic_always_lock_free ?
 Success(0, E) : Error(E);
   }
-  case Builtin::BIomp_is_initial_device:
-// We can decide statically which value the runtime would return if called.
-return Success(Info.getLangOpts().OpenMPIsDevice ? 0 : 1, E);
   case Builtin::BI__builtin_add_overflow:
   case Builtin::BI__builtin_sub_overflow:
   case Builtin::BI__builtin_mul_overflow:

diff  --git a/clang/test/OpenMP/is_initial_device.c 
b/clang/test/OpenMP/is_initial_device.c
deleted file mode 100644
index 2fe93a4eb6c46..0
--- a/clang/test/OpenMP/is_initial_device.c
+++ /dev/null
@@ -1,41 +0,0 @@
-// REQUIRES: powerpc-registered-target
-
-// RUN: %clang_cc1 -verify -fopenmp -x c -triple powerpc64le-unknown-unknown 
-fopenmp-targets=powerpc64le-unknown-unknown \
-// RUN:-emit-llvm-bc %s -o %t-ppc-host.bc
-// RUN: %clang_cc1 -verify -fopenmp -x ir -triple powerpc64le-unknown-unknown 
-emit-llvm \
-// RUN: %t-ppc-host.bc -o - | FileCheck %s -check-prefixes 
HOST,OUTLINED
-// RUN: %clang_cc1 -verify -fopenmp -x c -triple powerpc64le-unknown-unknown 
-emit-llvm -fopenmp-is-device \
-// RUN: %s -fopenmp-host-ir-file-path %t-ppc-host.bc -o - | 
FileCheck %s -check-prefixes DEVICE,OUTLINED
-
-// RUN: %clang_cc1 -verify -fopenmp-simd -x c -triple 
powerpc64le-unknown-unknown -fopenmp-targets=powerpc64le-unknown-unknown 
-emit-llvm-bc %s -o %t-ppc-host.bc
-// RUN: %clang_cc1 -verify -fopenmp-simd -x ir -triple 
powerpc64le-unknown-unknown -emit-llvm %t-ppc-host.bc -o - | FileCheck 
--check-prefix SIMD-ONLY0 %s
-// RUN: %clang_cc1 -verify -fopenmp-simd -x c -triple 
powerpc64le-unknown-unknown -emit-llvm -fopenmp-is-device %s 
-fopenmp-host-ir-file-path %t-ppc-host.bc -o - | FileCheck --check-prefix 
SIMD-ONLY0 %s
-// SIMD-ONLY0-NOT: {{__kmpc|__tgt}}
-
-// expected-no-diagnostics
-int check() {
-  int host = omp_is_initial_device();
-  int device;
-#pragma omp target map(tofrom: device)
-  {
-device = omp_is_initial_device();
-  }
-
-  return host + device;
-}
-
-// The host should get a value of 1:
-// HOST: define{{.*}} @check()
-// HOST: [[HOST:%.*]] = alloca i32
-// HOST: store i32 1, i32* [[HOST]]
-
-// OUTLINED: define{{.*}} @{{.*}}omp_offloading{{.*}}(i32*{{.*}} 
[[DEVICE_ARGUMENT:%.*]])
-// OUTLINED: [[DEVICE_ADDR_STORAGE:%.*]] = alloca i32*
-// OUTLINED: store i32* [[DEVICE_ARGUMENT]], i32** [[DEVICE_ADDR_STORAGE]]
-// OUTLINED: [[DEVICE_ADDR:%.*]] = load i32*, i32** [[DEVICE_ADDR_STORAGE]]
-
-// The outlined function that is called as fallback also runs on the host:
-// HOST: store i32 1, i32* [[DEVICE_ADDR]]
-
-// The device should get a value of 0:
-// DEVICE: store i32 0, i32* [[DEVICE_ADDR]]

diff  --git a/openmp/libomptarget/test/api/is_initial_device.c 
b/openmp/libomptarget/test/api/is_initial_device.c
new file mode 100644
index 0..78980d6316c6d
--- /dev/null
+++ b/openmp/libomptarget/test/api/is_initial_device.c
@@ -0,0 +1,30 @@
+// RUN: 

[PATCH] D99683: [HIP] Support ThinLTO

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added a comment.

In D99683#2672554 , @tejohnson wrote:

> This raises some higher level questions for me:
>
> First, how will you deal with other corner cases that won't or cannot be 
> imported right now? While enabling importing of noinline functions and 
> cranking up the threshold will get the majority of functions imported, there 
> are cases that we still won't import (functions/vars that are interposable, 
> certain funcs/vars that cannot be renamed, most non-const variables with 
> non-trivial initializers).

We will document the limitation of thinLTO support of HIP toolchain and 
recommend users not to use thinLTO in those corner cases.

> Second, force importing of everything transitively referenced defeats the 
> purpose of ThinLTO and would probably make it worse than regular LTO. The 
> main entry module will need to import everything transitively referenced from 
> there, so everything not dead in the binary, which should make that module 
> post importing equivalent to a regular LTO module. In addition, every other 
> module needs to transitively import everything referenced from those modules, 
> making them very large depending on how many leaf vs non-leaf functions and 
> variables they contain. What is the goal of doing ThinLTO in this case?

The objective is to improve optimization/codegen time by using multi-threads of 
thinLTO. For example, I have 10 modules each containing a kernel. In full LTO 
linking, I get one big module containing 10 kernels with all functions inlined, 
and I have one thread for optimization/codegen. With thinLTO, I get one kernel 
in each module, with all functions inlined. AMDGPU internalization and global 
DCE will remove functions not used by that kernel in each module. I will get 10 
threads, each doing optimization/codegen for one kernel. Theoretically, there 
could be 10 times speed up.


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

https://reviews.llvm.org/D99683

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

[PATCH] D93185: [docs] Use make_unique in FrontendAction example

[Nicolás Alvarez via Phabricator via cfe-commits]

nicolas17 added a comment.

I don't have commit access, please push this for me.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93185

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

[PATCH] D99893: [WIP] Replace std::forward & std::move by cast expressions during Sema

[Corentin Jabot via Phabricator via cfe-commits]

cor3ntin added a comment.

In D99893#2672258 , @rjmccall wrote:

> Our builtin logic is capable of recognizing user declarations as builtins 
> without any sort of explicit annotation in source.  That's how we recognize C 
> library builtins, for example.  As Richard points out, that logic isn't 
> really set up to do the right thing for namespaced declarations (or template 
> ones, for that matter), but that seems fixable.
>
> The namespace issue might be annoying because of the inline namespaces that 
> some standard libraries used for ABI versioning.

I managed to make a somewhat generic mechanism to mark functions declared in 
the namespace std ( and I do think we want there to be a declaration in the 
header always) as built-ins.
Currently it's using the number of parameters as a filter - which I think is 
reasonable ( the issue as Richard pointed out is that `std::move` is overloaded 
 with the algorithm version).

I'm now struggling to handle code generation and constant evaluation, but 
hopefully I will find a way to get there.

This would solve some of the issues (perfect code-gen / no body instantiation), 
but the declarations are still instantiated of course.
And I have a few folks asking me whether I could get rid of it. Unfortunately, 
I think the goals of preserving the AST as is today and getting rid of these 
template instantiations are contradictory.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99893

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

[PATCH] D99194: [analyzer] Fix body farm for Obj-C++ properties

[Artem Dergachev via Phabricator via cfe-commits]

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

Great, thanks!~


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99194

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

[PATCH] D99262: [analyzer] Fix dead store checker false positive

[Artem Dergachev via Phabricator via cfe-commits]

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

Looks great and I'm also curious about nested initializers.




Comment at: clang/lib/StaticAnalyzer/Checkers/DeadStoresChecker.cpp:420-421
+  // We should also allow defensive initialization of structs.
+  if (const auto *ILE =
+  dyn_cast(E->IgnoreParenCasts())) {
+// We can use exactly the same logic here.

vsavchenko wrote:
> martong wrote:
> > What about nested InitListExpr's?
> > ```
> > std::array a1{ {1, 2, 3} };
> > ```
> > 
> > ```
> > VarDecl 0x561b200333a0  col:20 a1 
> > 'std::array':'std::array' listinit
> > `-InitListExpr 0x561b20036d78  'std::array > 3>':'std::array'
> >   `-InitListExpr 0x561b20036dc0  'typename 
> > _AT_Type::_Type':'int [3]'
> > |-IntegerLiteral 0x561b20033408  'int' 1
> > |-IntegerLiteral 0x561b20033428  'int' 2
> > `-IntegerLiteral 0x561b20033448  'int' 3
> > ```
> I'm not sure that we'll report anything on that
We probably won't because it's a C++ object, even though it's an aggregate so 
we should probably warn(?) What about a plain C object like
```lang=c++
int x[2][2] = { { 0, 0 }, { 0, 0 } };
```
?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99262

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

[PATCH] D99683: [HIP] Support ThinLTO

[Teresa Johnson via Phabricator via cfe-commits]

tejohnson added a comment.

In D99683#2671727 , @yaxunl wrote:

> In D99683#2669136 , @yaxunl wrote:
>
>> In D99683#2669080 , @tejohnson 
>> wrote:
>>
>>> In D99683#2669047 , @yaxunl wrote:
>>>
 In D99683#2664674 , @tejohnson 
 wrote:

> I haven't looked extensively yet, but why import noinline functions? 
> Also, please add a patch description.

 AMDGPU backend does not support linking of object files containing 
 external symbols, i.e. one object file calling a function defined in 
 another object file. Therefore the LLVM module passed to AMDGPU backend 
 needs to contain definitions of all callees, even if a callee has noinline 
 attribute. To support backends like this, the function importer needs to 
 be able to import functions with noinline attribute. Therefore we add an 
 LLVM option for allowing that, which is off by default. We have comments 
 at line 70 of HIP.cpp about this.
>>>
>>> How does a non-LTO build work, or is (full) LTO currently required? Because 
>>> with ThinLTO we only import functions that are externally defined but 
>>> referenced in the current module. Also, when ThinLTO imports functions it 
>>> makes them available_externally, which means they are dropped and made 
>>> external symbols again after inlining. So anything imported but not inlined 
>>> will go back to being an external symbol.
>>
>> AMDGPU backend by default uses full LTO for linking. It does not support 
>> non-LTO linking. Currently, it inlines all functions except kernels. However 
>> we want to be able to be able not to inline all functions. Is it OK to add 
>> an LLVM option to mark imported functions as linkonce_odr so that AMDGPU 
>> backend can keep the definitions of the imported functions?
>
> Actually AMDGPU backend will internalize all non-kernel functions before 
> codegen. Those functions with available_externally linkage will have internal 
> linkage before codegen, therefore they will not be dropped.

This raises some higher level questions for me:

First, how will you deal with other corner cases that won't or cannot be 
imported right now? While enabling importing of noinline functions and cranking 
up the threshold will get the majority of functions imported, there are cases 
that we still won't import (functions/vars that are interposable, certain 
funcs/vars that cannot be renamed, most non-const variables with non-trivial 
initializers).

Second, force importing of everything transitively referenced defeats the 
purpose of ThinLTO and would probably make it worse than regular LTO. The main 
entry module will need to import everything transitively referenced from there, 
so everything not dead in the binary, which should make that module post 
importing equivalent to a regular LTO module. In addition, every other module 
needs to transitively import everything referenced from those modules, making 
them very large depending on how many leaf vs non-leaf functions and variables 
they contain. What is the goal of doing ThinLTO in this case?


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

https://reviews.llvm.org/D99683

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

[PATCH] D99993: bugprone-argument-comment: ignore name mismatches for decls from system headers

[George Burgess IV via Phabricator via cfe-commits]

george.burgess.iv created this revision.
george.burgess.iv added reviewers: hokein, gribozavr2.
george.burgess.iv added a project: clang.
Herald added a subscriber: jfb.
george.burgess.iv requested review of this revision.
Herald added a project: clang-tools-extra.

As of 2a3498e24f97d 
, we 
ignore parameter name mismatches for functions in the `std::` namespace, since 
those aren't standardized. It seems reasonable to extend this to all functions 
which are declared in system headers, since this lint can be a bit noisy 
otherwise (https://bugs.chromium.org/p/chromium/issues/detail?id=1191507).

I'm on the fence about whether this behavior should be governed by an option. I 
tended toward no in the current patch, since `std::` isn't, but I'm happy to 
add a `CheckSystemHeaderDecls` flag or similar if that seems better.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D3

Files:
  clang-tools-extra/clang-tidy/bugprone/ArgumentCommentCheck.cpp
  
clang-tools-extra/test/clang-tidy/checkers/Inputs/bugprone-argument-comment/header-with-decl.h
  
clang-tools-extra/test/clang-tidy/checkers/Inputs/bugprone-argument-comment/system-header-with-decl.h
  clang-tools-extra/test/clang-tidy/checkers/bugprone-argument-comment.cpp


Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-argument-comment.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/bugprone-argument-comment.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-argument-comment.cpp
@@ -1,4 +1,4 @@
-// RUN: %check_clang_tidy %s bugprone-argument-comment %t
+// RUN: %check_clang_tidy %s bugprone-argument-comment %t -- -- -I 
%S/Inputs/bugprone-argument-comment
 
 // FIXME: clang-tidy should provide a -verify mode to make writing these checks
 // easier and more accurate.
@@ -134,3 +134,20 @@
   std::swap(a, /*num=*/b);
 }
 } // namespace ignore_std_functions
+
+namespace regular_header {
+#include "header-with-decl.h"
+void test() {
+  my_header_function(/*not_arg=*/1);
+// CHECK-NOTES: [[@LINE-1]]:22: warning: argument name 'not_arg' in comment 
does not match parameter name 'arg'
+// CHECK-NOTES: header-with-decl.h:1:29: note: 'arg' declared here
+// CHECK-FIXES: my_header_function(/*not_arg=*/1);
+}
+} // namespace regular_header
+
+namespace system_header {
+#include "system-header-with-decl.h"
+void test() {
+  my_system_header_function(/*not_arg=*/1);
+}
+} // namespace system_header
Index: 
clang-tools-extra/test/clang-tidy/checkers/Inputs/bugprone-argument-comment/system-header-with-decl.h
===
--- /dev/null
+++ 
clang-tools-extra/test/clang-tidy/checkers/Inputs/bugprone-argument-comment/system-header-with-decl.h
@@ -0,0 +1,3 @@
+#pragma clang system_header
+
+void my_system_header_function(int arg);
Index: 
clang-tools-extra/test/clang-tidy/checkers/Inputs/bugprone-argument-comment/header-with-decl.h
===
--- /dev/null
+++ 
clang-tools-extra/test/clang-tidy/checkers/Inputs/bugprone-argument-comment/header-with-decl.h
@@ -0,0 +1 @@
+void my_header_function(int arg);
Index: clang-tools-extra/clang-tidy/bugprone/ArgumentCommentCheck.cpp
===
--- clang-tools-extra/clang-tidy/bugprone/ArgumentCommentCheck.cpp
+++ clang-tools-extra/clang-tidy/bugprone/ArgumentCommentCheck.cpp
@@ -20,10 +20,12 @@
 namespace tidy {
 namespace bugprone {
 namespace {
-AST_MATCHER(Decl, isFromStdNamespace) {
+AST_MATCHER(Decl, isFromStdNamespaceOrSystemHeader) {
   if (const auto *D = Node.getDeclContext()->getEnclosingNamespaceContext())
-return D->isStdNamespace();
-  return false;
+if (D->isStdNamespace())
+  return true;
+  return Node.getASTContext().getSourceManager().isInSystemHeader(
+  Node.getLocation());
 }
 } // namespace
 
@@ -66,13 +68,13 @@
// not specified by the standard, and standard library
// implementations in practice have to use reserved names to
// avoid conflicts with same-named macros.
-   unless(hasDeclaration(isFromStdNamespace(
-  .bind("expr"),
-  this);
-  Finder->addMatcher(
-  cxxConstructExpr(unless(hasDeclaration(isFromStdNamespace(
+   unless(hasDeclaration(isFromStdNamespaceOrSystemHeader(
   .bind("expr"),
   this);
+  Finder->addMatcher(cxxConstructExpr(unless(hasDeclaration(
+  isFromStdNamespaceOrSystemHeader(
+ .bind("expr"),
+ this);
 }
 
 static std::vector>


Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-argument-comment.cpp
===
--- 

[PATCH] D99790: [CGCall] Annotate `this` argument with alignment

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

I'll land this tomorrow if no one wants to object.
As discussed in D99791 , this is an obvious, 
and legal, bugfix.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99790

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

[PATCH] D99791: [CGCall] Annotate pointer argument with alignment

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri planned changes to this revision.
lebedev.ri added a comment.

@rjmccall thank you for taking a look!

In D99791#2670333 , @rjmccall wrote:

> The last major conversation we had about this was this RFC I sent out about 
> five years ago:
>
>   https://lists.llvm.org/pipermail/llvm-dev/2016-January/094012.html
>
> In that RFC, I specifically argued that we should not do this, and that it 
> should only be considered undefined behavior to actually access memory 
> through a misaligned pointer (for a particular definition of "access 
> memory").  At Apple, we have made that a promise to our internal users, so 
> even if we decide to do this, we will need to not do it on Darwin.  However, 
> as I remember it, the LLVM community did not reach a consensus to adopt my 
> recommendation, so in principle we still have the flexibility to start doing 
> this.  I continue to believe that doing so would be a mistake.  At any rate, 
> you should start by reading that thread.

Thank you for the pointer!
I can for sure provide an opt-out, however note that in the end it will cause 
performance regressions as compared to the current LLVM optimizations.
I will start with an UBSan part then.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99791

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

[PATCH] D90188: Add support for attribute 'using_if_exists'

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/test/SemaCXX/using-if-exists.cpp:79
+  using B::mf UIE; // expected-note {{using declaration annotated with 
'using_if_exists' here}}
+  using typename B::mt UIE; // expected-note 2 {{using declaration annotated 
with 'using_if_exists' here}}
+

erik.pilkington wrote:
> Quuxplusone wrote:
> > I notice there's a hard `error: 'using_if_exists' attribute cannot be 
> > applied to types` on
> > 
> > using B::operator int UIE;
> > 
> > Any thoughts on how to disambiguate that grammar?
> Hmm, that's an interesting case. I'm not sure what the right approach is. I 
> think it would be a bit awkward to decide where to attach the attribute 
> depending on what kind of entities the attribute could potentially apply to. 
> FWIW this isn't a semantic restriction, since you can always just use the 
> prefix syntax: `UIE using B::operator int;`. Maybe @aaron.ballman has some 
> thoughts here?
By my reading of the standard, an attribute in that position really does apply 
to the type `int` in that case.

https://eel.is/c++draft/namespace.udecl#nt:using-declarator
https://eel.is/c++draft/expr.prim.id.unqual#nt:unqualified-id
https://eel.is/c++draft/class.conv.fct#nt:conversion-function-id
https://eel.is/c++draft/class.conv.fct#nt:conversion-type-id
https://eel.is/c++draft/dcl.type.general#nt:type-specifier-seq
https://eel.is/c++draft/dcl.type.general#1.sentence-2

That said, I don't know if that's really *expected* behavior, so this might be 
a question for WG21.


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

https://reviews.llvm.org/D90188

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

[PATCH] D99235: [HIP] Change to code object v4

[Yaxun Liu via Phabricator via cfe-commits]

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

ping.


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

https://reviews.llvm.org/D99235

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

[clang-tools-extra] c060945 - [docs] Update documentation for bugprone-misplaced-widening-cast

[via cfe-commits]

Author: Vince Bridgers
Date: 2021-04-06T16:18:50-05:00
New Revision: c060945b23a1c54d4b2a053ff4b093a2277b303d

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

LOG: [docs] Update documentation for bugprone-misplaced-widening-cast

The default setting for CheckImplicitCasts was changed in
https://reviews.llvm.org/D32164 but the documentation was not updated.
This simple change just syncs the documentation with the behavior of
that checker.

Reviewed By: aaron.ballman

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

Added: 


Modified: 

clang-tools-extra/docs/clang-tidy/checks/bugprone-misplaced-widening-cast.rst

Removed: 




diff  --git 
a/clang-tools-extra/docs/clang-tidy/checks/bugprone-misplaced-widening-cast.rst 
b/clang-tools-extra/docs/clang-tidy/checks/bugprone-misplaced-widening-cast.rst
index 08c63c632eb2..cec49c55309a 100644
--- 
a/clang-tools-extra/docs/clang-tidy/checks/bugprone-misplaced-widening-cast.rst
+++ 
b/clang-tools-extra/docs/clang-tidy/checks/bugprone-misplaced-widening-cast.rst
@@ -62,4 +62,4 @@ Options
 
 .. option:: CheckImplicitCasts
 
-   If `true`, enables detection of implicit casts. Default is `true`.
+   If `true`, enables detection of implicit casts. Default is `false`.



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

[PATCH] D99898: [clang, test] Fix use of undef FileCheck var

[John McCall via Phabricator via cfe-commits]

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

Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99898

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

[PATCH] D90188: Add support for attribute 'using_if_exists'

[Erik Pilkington via Phabricator via cfe-commits]

erik.pilkington added a comment.

Ping!


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

https://reviews.llvm.org/D90188

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

[PATCH] D99353: [driver] Make `clang` warn rather then error on `flang` options

[Joachim Protze via Phabricator via cfe-commits]

protze.joachim added a comment.

In D99353#2669046 , @awarzynski wrote:

> Btw, how important are these aliases for you?

I can work with `-ffixed-line-length=132` where needed. It's just not obvious 
from `flang --help` that this is an alias for `-ffixed-line-length-132` (or the 
other way around). I only learned that by looking at LLVM source.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99353

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

[PATCH] D98747: Thread safety analysis: Don't warn about managed locks on join points

[Aaron Puchert via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGdfec26b186d2: Thread safety analysis: Dont warn about 
managed locks on join points (authored by aaronpuchert).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98747

Files:
  clang/lib/Analysis/ThreadSafety.cpp
  clang/test/SemaCXX/warn-thread-safety-analysis.cpp


Index: clang/test/SemaCXX/warn-thread-safety-analysis.cpp
===
--- clang/test/SemaCXX/warn-thread-safety-analysis.cpp
+++ clang/test/SemaCXX/warn-thread-safety-analysis.cpp
@@ -2595,6 +2595,7 @@
   if (c) {// test join point -- held/not held during release
 rlock.Release();
   }
+  // No warning on join point because the lock will be released by the scope 
object anyway.
 }
 
 void Foo::test3() {
@@ -2615,7 +2616,7 @@
   if (c) {
 rlock.Release();
   }
-  // no warning on join point for managed lock.
+  // No warning on join point because the lock will be released by the scope 
object anyway.
   rlock.Release();  // expected-warning {{releasing mutex 'mu_' that was not 
held}}
 }
 
@@ -2659,6 +2660,7 @@
 
 Mutex mu;
 int x GUARDED_BY(mu);
+bool b;
 
 void print(int);
 
@@ -2740,6 +2742,23 @@
   scope.Lock(); // expected-warning {{acquiring mutex 'mu' that is already 
held}}
 }
 
+void lockJoin() {
+  RelockableMutexLock scope(, DeferTraits{});
+  if (b)
+scope.Lock();
+  // No warning on join point because the lock will be released by the scope 
object anyway.
+  x = 2; // expected-warning {{writing variable 'x' requires holding mutex 
'mu' exclusively}}
+}
+
+void unlockJoin() {
+  RelockableMutexLock scope(, DeferTraits{});
+  scope.Lock();
+  if (b)
+scope.Unlock();
+  // No warning on join point because the lock will be released by the scope 
object anyway.
+  x = 2; // expected-warning {{writing variable 'x' requires holding mutex 
'mu' exclusively}}
+}
+
 void directUnlock() {
   RelockableExclusiveMutexLock scope();
   mu.Unlock();
@@ -2871,10 +2890,9 @@
 
 void join() EXCLUSIVE_LOCKS_REQUIRED(mu) {
   MutexUnlock scope();
-  if (c) {
-scope.Lock(); // expected-note{{mutex acquired here}}
-  }
-  // expected-warning@+1{{mutex 'mu' is not held on every path through here}}
+  if (c)
+scope.Lock();
+  // No warning on join point because the lock will be released by the scope 
object anyway.
   scope.Lock();
 }
 
Index: clang/lib/Analysis/ThreadSafety.cpp
===
--- clang/lib/Analysis/ThreadSafety.cpp
+++ clang/lib/Analysis/ThreadSafety.cpp
@@ -983,7 +983,7 @@
 } else {
   FSet.removeLock(FactMan, !Cp);
   FSet.addLock(FactMan,
-   std::make_unique(Cp, kind, loc));
+   std::make_unique(Cp, kind, loc, true));
 }
   }
 


Index: clang/test/SemaCXX/warn-thread-safety-analysis.cpp
===
--- clang/test/SemaCXX/warn-thread-safety-analysis.cpp
+++ clang/test/SemaCXX/warn-thread-safety-analysis.cpp
@@ -2595,6 +2595,7 @@
   if (c) {// test join point -- held/not held during release
 rlock.Release();
   }
+  // No warning on join point because the lock will be released by the scope object anyway.
 }
 
 void Foo::test3() {
@@ -2615,7 +2616,7 @@
   if (c) {
 rlock.Release();
   }
-  // no warning on join point for managed lock.
+  // No warning on join point because the lock will be released by the scope object anyway.
   rlock.Release();  // expected-warning {{releasing mutex 'mu_' that was not held}}
 }
 
@@ -2659,6 +2660,7 @@
 
 Mutex mu;
 int x GUARDED_BY(mu);
+bool b;
 
 void print(int);
 
@@ -2740,6 +2742,23 @@
   scope.Lock(); // expected-warning {{acquiring mutex 'mu' that is already held}}
 }
 
+void lockJoin() {
+  RelockableMutexLock scope(, DeferTraits{});
+  if (b)
+scope.Lock();
+  // No warning on join point because the lock will be released by the scope object anyway.
+  x = 2; // expected-warning {{writing variable 'x' requires holding mutex 'mu' exclusively}}
+}
+
+void unlockJoin() {
+  RelockableMutexLock scope(, DeferTraits{});
+  scope.Lock();
+  if (b)
+scope.Unlock();
+  // No warning on join point because the lock will be released by the scope object anyway.
+  x = 2; // expected-warning {{writing variable 'x' requires holding mutex 'mu' exclusively}}
+}
+
 void directUnlock() {
   RelockableExclusiveMutexLock scope();
   mu.Unlock();
@@ -2871,10 +2890,9 @@
 
 void join() EXCLUSIVE_LOCKS_REQUIRED(mu) {
   MutexUnlock scope();
-  if (c) {
-scope.Lock(); // expected-note{{mutex acquired here}}
-  }
-  // expected-warning@+1{{mutex 'mu' is not held on every path through here}}
+  if (c)
+scope.Lock();
+  // No warning on join point because the lock will be released by the scope object anyway.
   

[clang] dfec26b - Thread safety analysis: Don't warn about managed locks on join points

[Aaron Puchert via cfe-commits]

Author: Aaron Puchert
Date: 2021-04-06T22:29:48+02:00
New Revision: dfec26b186d2f0c80f2b70901b7cc5747f5b377c

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

LOG: Thread safety analysis: Don't warn about managed locks on join points

We already did so for scoped locks acquired in the constructor, this
change extends the treatment to deferred locks and scoped unlocking, so
locks acquired outside of the constructor. Obviously this makes things
more consistent.

Originally I thought this was a bad idea, because obviously it
introduces false negatives when it comes to double locking, but these
are typically easily found in tests, and the primary goal of the Thread
safety analysis is not to find double locks but race conditions.
Since the scoped lock will release the mutex anyway when the scope ends,
the inconsistent state is just temporary and probably fine.

Reviewed By: delesley

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

Added: 


Modified: 
clang/lib/Analysis/ThreadSafety.cpp
clang/test/SemaCXX/warn-thread-safety-analysis.cpp

Removed: 




diff  --git a/clang/lib/Analysis/ThreadSafety.cpp 
b/clang/lib/Analysis/ThreadSafety.cpp
index 84e0e91f597fe..00678c7129a31 100644
--- a/clang/lib/Analysis/ThreadSafety.cpp
+++ b/clang/lib/Analysis/ThreadSafety.cpp
@@ -983,7 +983,7 @@ class ScopedLockableFactEntry : public FactEntry {
 } else {
   FSet.removeLock(FactMan, !Cp);
   FSet.addLock(FactMan,
-   std::make_unique(Cp, kind, loc));
+   std::make_unique(Cp, kind, loc, true));
 }
   }
 

diff  --git a/clang/test/SemaCXX/warn-thread-safety-analysis.cpp 
b/clang/test/SemaCXX/warn-thread-safety-analysis.cpp
index d1520b1decbd3..b837206138a67 100644
--- a/clang/test/SemaCXX/warn-thread-safety-analysis.cpp
+++ b/clang/test/SemaCXX/warn-thread-safety-analysis.cpp
@@ -2595,6 +2595,7 @@ void Foo::test2() {
   if (c) {// test join point -- held/not held during release
 rlock.Release();
   }
+  // No warning on join point because the lock will be released by the scope 
object anyway.
 }
 
 void Foo::test3() {
@@ -2615,7 +2616,7 @@ void Foo::test5() {
   if (c) {
 rlock.Release();
   }
-  // no warning on join point for managed lock.
+  // No warning on join point because the lock will be released by the scope 
object anyway.
   rlock.Release();  // expected-warning {{releasing mutex 'mu_' that was not 
held}}
 }
 
@@ -2659,6 +2660,7 @@ class SCOPED_LOCKABLE RelockableMutexLock {
 
 Mutex mu;
 int x GUARDED_BY(mu);
+bool b;
 
 void print(int);
 
@@ -2740,6 +2742,23 @@ void doubleLock2() {
   scope.Lock(); // expected-warning {{acquiring mutex 'mu' that is already 
held}}
 }
 
+void lockJoin() {
+  RelockableMutexLock scope(, DeferTraits{});
+  if (b)
+scope.Lock();
+  // No warning on join point because the lock will be released by the scope 
object anyway.
+  x = 2; // expected-warning {{writing variable 'x' requires holding mutex 
'mu' exclusively}}
+}
+
+void unlockJoin() {
+  RelockableMutexLock scope(, DeferTraits{});
+  scope.Lock();
+  if (b)
+scope.Unlock();
+  // No warning on join point because the lock will be released by the scope 
object anyway.
+  x = 2; // expected-warning {{writing variable 'x' requires holding mutex 
'mu' exclusively}}
+}
+
 void directUnlock() {
   RelockableExclusiveMutexLock scope();
   mu.Unlock();
@@ -2871,10 +2890,9 @@ void manual() EXCLUSIVE_LOCKS_REQUIRED(mu) {
 
 void join() EXCLUSIVE_LOCKS_REQUIRED(mu) {
   MutexUnlock scope();
-  if (c) {
-scope.Lock(); // expected-note{{mutex acquired here}}
-  }
-  // expected-warning@+1{{mutex 'mu' is not held on every path through here}}
+  if (c)
+scope.Lock();
+  // No warning on join point because the lock will be released by the scope 
object anyway.
   scope.Lock();
 }
 



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

[PATCH] D99675: RFC [llvm][clang] Create new intrinsic llvm.arith.fence to control FP optimization at expression level

[Andy Kaylor via Phabricator via cfe-commits]

andrew.w.kaylor added a comment.

In D99675#2671924 , @efriedma wrote:

>> The expression “llvm.arith.fence(a * b) + c” means that “a * b” must happen 
>> before “+ c” and FMA guarantees that, but to prevent later optimizations 
>> from unpacking the FMA the correct transformation needs to be:
>>
>> llvm.arith.fence(a * b) + c  →  llvm.arith.fence(FMA(a, b, c))
>
> Does this actually block later transforms from unpacking the FMA?  Maybe if 
> the FMA isn't marked "fast"...

Later transforms could unpack the FMA, but the result would be fenced. The 
intent isn't so much to prevent the FMA from being unpacked as to prevent 
losing the original fence semantics. That said, it doesn't quite work. For 
example, you might have this:

  %mul = fmul fast float %a, %b
  %fenced_mul = call float @llvm.arith.fence.f32(%mul)
  %result = fadd fast float %fenced_mul, %c

If there are no other uses of %fenced_mul, that could become

  %tmp = call fast float @llvm.fmuladd.f32(float %a, float %b, float %c)
  %result = call float @llvm.arith.fence.f32(%tmp)

If a later optimization decided to unpack this, it would become this:

  %mul = fmul fast float %a, %b
  %tmp = fadd fast float %mul, %c
  %result = call float @llvm.arith.fence.f32(%tmp)

I suggested this as a way of enabling the FMA optimization. It brings the fadd 
into the fence, but still protects the fmul from being reassociated or 
otherwise transformed with other operations outside the fence. In a purely 
practical sense, this would probably work. In a more strict sense, though, I 
now see that it has the problem that you could legally distribute the addition 
within the fence. I can't see a practical reason anyone would do that, but the 
semantics would allow it. The same ("legal but not practical") is true of 
forming the fmuladd intrinsic before codegen, I think.

So, no, I don't think this works the way it was intended.

That might push us back to Kevin's suggestion of just not allowing the FMA 
optimization across a fence.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99675

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

[PATCH] D99037: [Matrix] Implement C-style explicit type conversions for matrix types

[Saurabh Jha via Phabricator via cfe-commits]

SaurabhJha updated this revision to Diff 335643.
SaurabhJha added a comment.

Changes in latest revision:

- Updated definition of areMatrixTypesOfTheSameDimension to reflect the comment
- Refactored casting between types into EmitCastBetweenScalarTypes
- Removed mentions of "non matrix"
- Replaced matrices with matrixes
- Update error messages to "..is not allowed"


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99037

Files:
  clang/include/clang/AST/OperationKinds.def
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/Expr.cpp
  clang/lib/AST/ExprConstant.cpp
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/CodeGen/CGExprAgg.cpp
  clang/lib/CodeGen/CGExprComplex.cpp
  clang/lib/CodeGen/CGExprConstant.cpp
  clang/lib/CodeGen/CGExprScalar.cpp
  clang/lib/Edit/RewriteObjCFoundationAPI.cpp
  clang/lib/Sema/SemaCast.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp
  clang/test/CodeGen/matrix-cast.c
  clang/test/Sema/matrix-cast.c
  clang/test/SemaCXX/matrix-casts.cpp

Index: clang/test/SemaCXX/matrix-casts.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/matrix-casts.cpp
@@ -0,0 +1,67 @@
+// RUN: %clang_cc1 -std=c++11 -fenable-matrix -fsyntax-only -verify %s
+
+template 
+
+using matrix_4_4 = X __attribute__((matrix_type(4, 4)));
+
+template 
+
+using matrix_5_5 = Y __attribute__((matrix_type(5, 5)));
+
+typedef struct test_struct {
+} test_struct;
+
+typedef int vec __attribute__((vector_size(4)));
+
+void f1() {
+  // TODO: Update this test once the support of C-style casts for C++ is implemented.
+  matrix_4_4 m1;
+  matrix_4_4 m2;
+  matrix_4_4 m3;
+  matrix_5_5 m4;
+  int i;
+  vec v;
+  test_struct *s;
+
+  (matrix_4_4)m1; // expected-error {{C-style cast from 'matrix_4_4' (aka 'char __attribute__((matrix_type(4, \
+4)))') to 'matrix_4_4' (aka 'int __attribute__((matrix_type(4, 4)))') is not allowed}}
+  (matrix_4_4)m2; // expected-error {{C-style cast from 'matrix_4_4' (aka 'int __attribute__((matrix_type(4, \
+4)))') to 'matrix_4_4' (aka 'short __attribute__((matrix_type(4, 4)))') is not allowed}}
+  (matrix_5_5)m3; // expected-error {{C-style cast from 'matrix_4_4' (aka 'short __attribute__((matrix_type(4, \
+4)))') to 'matrix_5_5' (aka 'int __attribute__((matrix_type(5, 5)))') is not allowed}}
+
+  (int)m3; // expected-error {{C-style cast from 'matrix_4_4' (aka 'short __attribute__((matrix_type(4, \
+4)))') to 'int'}}
+  (matrix_4_4)i; // expected-error {{C-style cast from 'int' to 'matrix_4_4' (aka 'int __attribute__((\
+matrix_type(4, 4)))') is not allowed}}
+
+  (vec)m2; // expected-error {{C-style cast from 'matrix_4_4' (aka 'int __attribute__((matrix_type(4, 4)))') \
+to 'vec' (vector of 1 'int' value) is not allowed}}
+  (matrix_4_4)v; // expected-error {{C-style cast from 'vec' (vector of 1 'int' value) to 'matrix_4_4' \
+(aka 'char __attribute__((matrix_type(4, 4)))') is not allowed}}
+
+  (test_struct *)m1; // expected-error {{cannot cast from type 'matrix_4_4' (aka 'char __attribute__\
+((matrix_type(4, 4)))') to pointer type 'test_struct *}}'
+  (matrix_5_5)s; // expected-error {{C-style cast from 'test_struct *' to 'matrix_5_5' (aka 'float __attribute__\
+((matrix_type(5, 5)))') is not allowed}}'
+}
+
+void f2() {
+  // TODO: Update this test once the support of C-style casts for C++ is implemented.
+  matrix_4_4 m1;
+  matrix_5_5 m2;
+  matrix_5_5 m3;
+  matrix_4_4 m4;
+  float f;
+
+  (matrix_4_4)m1; // expected-error {{C-style cast from 'matrix_4_4' (aka 'float __attribute__\
+((matrix_type(4, 4)))') to 'matrix_4_4' (aka 'double __attribute__((matrix_type(4, 4)))') is not allowed}}
+  (matrix_5_5)m2; // expected-error {{C-style cast from 'matrix_5_5' (aka 'double __attribute__\
+((matrix_type(5, 5)))') to 'matrix_5_5' (aka 'float __attribute__((matrix_type(5, 5)))') is not allowed}}
+  (matrix_5_5)m3; // expected-error {{C-style cast from 'matrix_5_5' (aka 'int __attribute__\
+((matrix_type(5, 5)))') to 'matrix_5_5' (aka 'unsigned int __attribute__((matrix_type(5, 5)))') \
+is not allowed}}
+  (matrix_4_4)m4; // expected-error {{C-style cast from 'matrix_4_4' (aka 'unsigned int \
+__attribute__((matrix_type(4, 4)))') to 'matrix_4_4' (aka 'int __attribute__((matrix_type(4, 4)))') is not \
+allowed}}
+}
Index: clang/test/Sema/matrix-cast.c
===
--- /dev/null
+++ clang/test/Sema/matrix-cast.c
@@ -0,0 +1,75 @@
+// RUN: %clang_cc1 -fenable-matrix -fsyntax-only %s -verify
+
+typedef char cx4x4 __attribute__((matrix_type(4, 4)));
+typedef int ix4x4 __attribute__((matrix_type(4, 4)));
+typedef short sx4x4 __attribute__((matrix_type(4, 4)));
+typedef int ix5x5 __attribute__((matrix_type(5, 5)));
+typedef float fx5x5 __attribute__((matrix_type(5, 5)));
+typedef int vec __attribute__((vector_size(4)));
+typedef 

[PATCH] D99898: [clang, test] Fix use of undef FileCheck var

[Thomas Preud'homme via Phabricator via cfe-commits]

thopre updated this revision to Diff 335640.
thopre added a comment.

Rename NUW_RN for CHECK-YES to NUW


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99898

Files:
  clang/test/CodeGen/libcalls.c


Index: clang/test/CodeGen/libcalls.c
===
--- clang/test/CodeGen/libcalls.c
+++ clang/test/CodeGen/libcalls.c
@@ -88,9 +88,9 @@
 // CHECK-NO: declare double @atan(double) [[NUW_RN:#[0-9]+]]
 // CHECK-NO: declare x86_fp80 @atanl(x86_fp80) [[NUW_RN]]
 // CHECK-NO: declare float @atanf(float) [[NUW_RN]]
-// CHECK-YES-NOT: declare double @atan(double) [[NUW_RN]]
-// CHECK-YES-NOT: declare x86_fp80 @atanl(x86_fp80) [[NUW_RN]]
-// CHECK-YES-NOT: declare float @atanf(float) [[NUW_RN]]
+// CHECK-YES: declare double @atan(double) [[NUW:#[0-9]+]]
+// CHECK-YES: declare x86_fp80 @atanl(x86_fp80) [[NUW]]
+// CHECK-YES: declare float @atanf(float) [[NUW]]
 
   double atan2_ = atan2(d, 2);
   long double atan2l_ = atan2l(ld, ld);
@@ -98,9 +98,9 @@
 // CHECK-NO: declare double @atan2(double, double) [[NUW_RN]]
 // CHECK-NO: declare x86_fp80 @atan2l(x86_fp80, x86_fp80) [[NUW_RN]]
 // CHECK-NO: declare float @atan2f(float, float) [[NUW_RN]]
-// CHECK-YES-NOT: declare double @atan2(double, double) [[NUW_RN]]
-// CHECK-YES-NOT: declare x86_fp80 @atan2l(x86_fp80, x86_fp80) [[NUW_RN]]
-// CHECK-YES-NOT: declare float @atan2f(float, float) [[NUW_RN]]
+// CHECK-YES: declare double @atan2(double, double) [[NUW]]
+// CHECK-YES: declare x86_fp80 @atan2l(x86_fp80, x86_fp80) [[NUW]]
+// CHECK-YES: declare float @atan2f(float, float) [[NUW]]
 
   double exp_ = exp(d);
   long double expl_ = expl(ld);
@@ -108,9 +108,9 @@
 // CHECK-NO: declare double @llvm.exp.f64(double) [[NUW_RNI]]
 // CHECK-NO: declare x86_fp80 @llvm.exp.f80(x86_fp80) [[NUW_RNI]]
 // CHECK-NO: declare float @llvm.exp.f32(float) [[NUW_RNI]]
-// CHECK-YES-NOT: declare double @exp(double) [[NUW_RN]]
-// CHECK-YES-NOT: declare x86_fp80 @expl(x86_fp80) [[NUW_RN]]
-// CHECK-YES-NOT: declare float @expf(float) [[NUW_RN]]
+// CHECK-YES: declare double @exp(double) [[NUW]]
+// CHECK-YES: declare x86_fp80 @expl(x86_fp80) [[NUW]]
+// CHECK-YES: declare float @expf(float) [[NUW]]
 
   double log_ = log(d);
   long double logl_ = logl(ld);
@@ -118,10 +118,11 @@
 // CHECK-NO: declare double @llvm.log.f64(double) [[NUW_RNI]]
 // CHECK-NO: declare x86_fp80 @llvm.log.f80(x86_fp80) [[NUW_RNI]]
 // CHECK-NO: declare float @llvm.log.f32(float) [[NUW_RNI]]
-// CHECK-YES-NOT: declare double @log(double) [[NUW_RN]]
-// CHECK-YES-NOT: declare x86_fp80 @logl(x86_fp80) [[NUW_RN]]
-// CHECK-YES-NOT: declare float @logf(float) [[NUW_RN]]
+// CHECK-YES: declare double @log(double) [[NUW]]
+// CHECK-YES: declare x86_fp80 @logl(x86_fp80) [[NUW]]
+// CHECK-YES: declare float @logf(float) [[NUW]]
 }
 
+// CHECK-YES: attributes [[NUW]] = { nounwind "frame-pointer"="none" 
"no-trapping-math"="true" "stack-protector-buffer-size"="8" 
"target-features"="+cx8,+x87" }
 // CHECK-NO-DAG: attributes [[NUW_RN]] = { nounwind readnone{{.*}} }
 // CHECK-NO-DAG: attributes [[NUW_RNI]] = { nofree nosync nounwind readnone 
speculatable willreturn }


Index: clang/test/CodeGen/libcalls.c
===
--- clang/test/CodeGen/libcalls.c
+++ clang/test/CodeGen/libcalls.c
@@ -88,9 +88,9 @@
 // CHECK-NO: declare double @atan(double) [[NUW_RN:#[0-9]+]]
 // CHECK-NO: declare x86_fp80 @atanl(x86_fp80) [[NUW_RN]]
 // CHECK-NO: declare float @atanf(float) [[NUW_RN]]
-// CHECK-YES-NOT: declare double @atan(double) [[NUW_RN]]
-// CHECK-YES-NOT: declare x86_fp80 @atanl(x86_fp80) [[NUW_RN]]
-// CHECK-YES-NOT: declare float @atanf(float) [[NUW_RN]]
+// CHECK-YES: declare double @atan(double) [[NUW:#[0-9]+]]
+// CHECK-YES: declare x86_fp80 @atanl(x86_fp80) [[NUW]]
+// CHECK-YES: declare float @atanf(float) [[NUW]]
 
   double atan2_ = atan2(d, 2);
   long double atan2l_ = atan2l(ld, ld);
@@ -98,9 +98,9 @@
 // CHECK-NO: declare double @atan2(double, double) [[NUW_RN]]
 // CHECK-NO: declare x86_fp80 @atan2l(x86_fp80, x86_fp80) [[NUW_RN]]
 // CHECK-NO: declare float @atan2f(float, float) [[NUW_RN]]
-// CHECK-YES-NOT: declare double @atan2(double, double) [[NUW_RN]]
-// CHECK-YES-NOT: declare x86_fp80 @atan2l(x86_fp80, x86_fp80) [[NUW_RN]]
-// CHECK-YES-NOT: declare float @atan2f(float, float) [[NUW_RN]]
+// CHECK-YES: declare double @atan2(double, double) [[NUW]]
+// CHECK-YES: declare x86_fp80 @atan2l(x86_fp80, x86_fp80) [[NUW]]
+// CHECK-YES: declare float @atan2f(float, float) [[NUW]]
 
   double exp_ = exp(d);
   long double expl_ = expl(ld);
@@ -108,9 +108,9 @@
 // CHECK-NO: declare double @llvm.exp.f64(double) [[NUW_RNI]]
 // CHECK-NO: declare x86_fp80 @llvm.exp.f80(x86_fp80) [[NUW_RNI]]
 // CHECK-NO: declare float @llvm.exp.f32(float) [[NUW_RNI]]
-// CHECK-YES-NOT: declare double @exp(double) [[NUW_RN]]
-// CHECK-YES-NOT: declare 

[PATCH] D97653: [clang-tidy] Fix RenamerClangTidy checks breaking lambda captures.

[Nathan James via Phabricator via cfe-commits]

njames93 added a comment.

Ping?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97653

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

[PATCH] D71726: Let clang atomic builtins fetch add/sub support floating point types

[Yaxun Liu via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG61d065e21ff3: Let clang atomic builtins fetch add/sub 
support floating point types (authored by yaxunl).
Herald added a project: clang.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71726

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/CodeGen/CGAtomic.cpp
  clang/lib/Sema/SemaChecking.cpp
  clang/test/CodeGen/fp-atomic-ops.c
  clang/test/CodeGenCUDA/amdgpu-atomic-ops.cu
  clang/test/CodeGenOpenCL/atomic-ops.cl
  clang/test/Sema/atomic-ops.c
  clang/test/SemaOpenCL/atomic-ops.cl

Index: clang/test/SemaOpenCL/atomic-ops.cl
===
--- clang/test/SemaOpenCL/atomic-ops.cl
+++ clang/test/SemaOpenCL/atomic-ops.cl
@@ -1,10 +1,13 @@
-// RUN: %clang_cc1 %s -cl-std=CL2.0 -verify -fsyntax-only -triple=spir64
-// RUN: %clang_cc1 %s -cl-std=CL2.0 -verify -fsyntax-only -triple=amdgcn-amdhsa-amd-opencl
+// RUN: %clang_cc1 %s -cl-std=CL2.0 -verify=expected,spir \
+// RUN:   -fsyntax-only -triple=spir64
+// RUN: %clang_cc1 %s -cl-std=CL2.0 -verify -fsyntax-only \
+// RUN:   -triple=amdgcn-amd-amdhsa
 
 // Basic parsing/Sema tests for __opencl_atomic_*
 
 #pragma OPENCL EXTENSION cl_khr_int64_base_atomics : enable
 #pragma OPENCL EXTENSION cl_khr_int64_extended_atomics : enable
+#pragma OPENCL EXTENSION cl_khr_fp16 : enable
 
 typedef __INTPTR_TYPE__ intptr_t;
 typedef int int8 __attribute__((ext_vector_type(8)));
@@ -36,7 +39,7 @@
 
 atomic_int gn;
 void f(atomic_int *i, const atomic_int *ci,
-   atomic_intptr_t *p, atomic_float *d,
+   atomic_intptr_t *p, atomic_float *f, atomic_double *d, atomic_half *h, // expected-error {{unknown type name 'atomic_half'}}
int *I, const int *CI,
intptr_t *P, float *D, struct S *s1, struct S *s2,
global atomic_int *i_g, local atomic_int *i_l, private atomic_int *i_p,
@@ -57,37 +60,38 @@
 
   __opencl_atomic_load(i, memory_order_seq_cst, memory_scope_work_group);
   __opencl_atomic_load(p, memory_order_seq_cst, memory_scope_work_group);
-  __opencl_atomic_load(d, memory_order_seq_cst, memory_scope_work_group);
+  __opencl_atomic_load(f, memory_order_seq_cst, memory_scope_work_group);
   __opencl_atomic_load(ci, memory_order_seq_cst, memory_scope_work_group);
   __opencl_atomic_load(i_c, memory_order_seq_cst, memory_scope_work_group); // expected-error {{address argument to atomic operation must be a pointer to non-constant _Atomic type ('__constant atomic_int *' (aka '__constant _Atomic(int) *') invalid)}}
 
   __opencl_atomic_store(i, 1, memory_order_seq_cst, memory_scope_work_group);
   __opencl_atomic_store(p, 1, memory_order_seq_cst, memory_scope_work_group);
-  (int)__opencl_atomic_store(d, 1, memory_order_seq_cst, memory_scope_work_group); // expected-error {{operand of type 'void' where arithmetic or pointer type is required}}
+  (int)__opencl_atomic_store(f, 1, memory_order_seq_cst, memory_scope_work_group); // expected-error {{operand of type 'void' where arithmetic or pointer type is required}}
 
   int exchange_1 = __opencl_atomic_exchange(i, 1, memory_order_seq_cst, memory_scope_work_group);
   int exchange_2 = __opencl_atomic_exchange(I, 1, memory_order_seq_cst, memory_scope_work_group); // expected-error {{address argument to atomic operation must be a pointer to _Atomic}}
 
   __opencl_atomic_fetch_add(i, 1, memory_order_seq_cst, memory_scope_work_group);
   __opencl_atomic_fetch_add(p, 1, memory_order_seq_cst, memory_scope_work_group);
-  __opencl_atomic_fetch_add(d, 1, memory_order_seq_cst, memory_scope_work_group); // expected-error {{address argument to atomic operation must be a pointer to atomic integer or pointer ('__generic atomic_float *' (aka '__generic _Atomic(float) *') invalid)}}
+  __opencl_atomic_fetch_add(f, 1.0f, memory_order_seq_cst, memory_scope_work_group);
+  __opencl_atomic_fetch_add(d, 1.0, memory_order_seq_cst, memory_scope_work_group);
   __opencl_atomic_fetch_and(i, 1, memory_order_seq_cst, memory_scope_work_group);
   __opencl_atomic_fetch_and(p, 1, memory_order_seq_cst, memory_scope_work_group);
-  __opencl_atomic_fetch_and(d, 1, memory_order_seq_cst, memory_scope_work_group); // expected-error {{address argument to atomic operation must be a pointer to atomic integer ('__generic atomic_float *' (aka '__generic _Atomic(float) *') invalid)}}
+  __opencl_atomic_fetch_and(f, 1, memory_order_seq_cst, memory_scope_work_group); // expected-error {{address argument to atomic operation must be a pointer to atomic integer ('__generic atomic_float *' (aka '__generic _Atomic(float) *') invalid)}}
 
   __opencl_atomic_fetch_min(i, 1, memory_order_seq_cst, memory_scope_work_group);
   __opencl_atomic_fetch_max(i, 1, memory_order_seq_cst, memory_scope_work_group);
-  __opencl_atomic_fetch_min(d, 1, 

[clang] 61d065e - Let clang atomic builtins fetch add/sub support floating point types

[Yaxun Liu via cfe-commits]

Author: Yaxun (Sam) Liu
Date: 2021-04-06T15:44:00-04:00
New Revision: 61d065e21ff37fb9040aed711c97daddac2f7577

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

LOG: Let clang atomic builtins fetch add/sub support floating point types

Recently atomicrmw started to support fadd/fsub:

https://reviews.llvm.org/D53965

However clang atomic builtins fetch add/sub still does not support
emitting atomicrmw fadd/fsub.

This patch adds that.

Reviewed by: John McCall, Artem Belevich, Matt Arsenault, JF Bastien,
James Y Knight, Louis Dionne, Olivier Giroux

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

Added: 
clang/test/CodeGen/fp-atomic-ops.c
clang/test/CodeGenCUDA/amdgpu-atomic-ops.cu

Modified: 
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/lib/CodeGen/CGAtomic.cpp
clang/lib/Sema/SemaChecking.cpp
clang/test/CodeGenOpenCL/atomic-ops.cl
clang/test/Sema/atomic-ops.c
clang/test/SemaOpenCL/atomic-ops.cl

Removed: 




diff  --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td 
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 2d43f5d3dfa49..0e14f8204f566 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -8188,6 +8188,9 @@ def err_atomic_op_needs_non_const_pointer : Error<
 def err_atomic_op_needs_trivial_copy : Error<
   "address argument to atomic operation must be a pointer to a "
   "trivially-copyable type (%0 invalid)">;
+def err_atomic_op_needs_atomic_int_ptr_or_fp : Error<
+  "address argument to atomic operation must be a pointer to %select{|atomic 
}0"
+  "integer, pointer or supported floating point type (%1 invalid)">;
 def err_atomic_op_needs_atomic_int_or_ptr : Error<
   "address argument to atomic operation must be a pointer to %select{|atomic 
}0"
   "integer or pointer (%1 invalid)">;

diff  --git a/clang/lib/CodeGen/CGAtomic.cpp b/clang/lib/CodeGen/CGAtomic.cpp
index 8ac36e4a6b64b..ef016cf24f134 100644
--- a/clang/lib/CodeGen/CGAtomic.cpp
+++ b/clang/lib/CodeGen/CGAtomic.cpp
@@ -602,21 +602,25 @@ static void EmitAtomicOp(CodeGenFunction , AtomicExpr 
*E, Address Dest,
 break;
 
   case AtomicExpr::AO__atomic_add_fetch:
-PostOp = llvm::Instruction::Add;
+PostOp = E->getValueType()->isFloatingType() ? llvm::Instruction::FAdd
+ : llvm::Instruction::Add;
 LLVM_FALLTHROUGH;
   case AtomicExpr::AO__c11_atomic_fetch_add:
   case AtomicExpr::AO__opencl_atomic_fetch_add:
   case AtomicExpr::AO__atomic_fetch_add:
-Op = llvm::AtomicRMWInst::Add;
+Op = E->getValueType()->isFloatingType() ? llvm::AtomicRMWInst::FAdd
+ : llvm::AtomicRMWInst::Add;
 break;
 
   case AtomicExpr::AO__atomic_sub_fetch:
-PostOp = llvm::Instruction::Sub;
+PostOp = E->getValueType()->isFloatingType() ? llvm::Instruction::FSub
+ : llvm::Instruction::Sub;
 LLVM_FALLTHROUGH;
   case AtomicExpr::AO__c11_atomic_fetch_sub:
   case AtomicExpr::AO__opencl_atomic_fetch_sub:
   case AtomicExpr::AO__atomic_fetch_sub:
-Op = llvm::AtomicRMWInst::Sub;
+Op = E->getValueType()->isFloatingType() ? llvm::AtomicRMWInst::FSub
+ : llvm::AtomicRMWInst::Sub;
 break;
 
   case AtomicExpr::AO__atomic_min_fetch:
@@ -813,6 +817,8 @@ RValue CodeGenFunction::EmitAtomicExpr(AtomicExpr *E) {
   bool Oversized = getContext().toBits(TInfo.Width) > MaxInlineWidthInBits;
   bool Misaligned = (Ptr.getAlignment() % TInfo.Width) != 0;
   bool UseLibcall = Misaligned | Oversized;
+  bool ShouldCastToIntPtrTy = true;
+
   CharUnits MaxInlineWidth =
   getContext().toCharUnitsFromBits(MaxInlineWidthInBits);
 
@@ -892,11 +898,14 @@ RValue CodeGenFunction::EmitAtomicExpr(AtomicExpr *E) {
   EmitStoreOfScalar(Val1Scalar, MakeAddrLValue(Temp, Val1Ty));
   break;
 }
-  LLVM_FALLTHROUGH;
+LLVM_FALLTHROUGH;
   case AtomicExpr::AO__atomic_fetch_add:
   case AtomicExpr::AO__atomic_fetch_sub:
   case AtomicExpr::AO__atomic_add_fetch:
   case AtomicExpr::AO__atomic_sub_fetch:
+ShouldCastToIntPtrTy = !MemTy->isFloatingType();
+LLVM_FALLTHROUGH;
+
   case AtomicExpr::AO__c11_atomic_store:
   case AtomicExpr::AO__c11_atomic_exchange:
   case AtomicExpr::AO__opencl_atomic_store:
@@ -937,15 +946,23 @@ RValue CodeGenFunction::EmitAtomicExpr(AtomicExpr *E) {
   LValue AtomicVal = MakeAddrLValue(Ptr, AtomicTy);
   AtomicInfo Atomics(*this, AtomicVal);
 
-  Ptr = Atomics.emitCastToAtomicIntPointer(Ptr);
-  if (Val1.isValid()) Val1 = Atomics.convertToAtomicIntPointer(Val1);
-  if (Val2.isValid()) Val2 = Atomics.convertToAtomicIntPointer(Val2);
-  if (Dest.isValid())
-Dest = 

[PATCH] D98738: [clang-tidy] performance-* checks: Match AllowedTypes against qualified type names when they contain "::".

[Felix Berger via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGddebed8e9742: [clang-tidy] performance-* checks: Match 
AllowedTypes against qualified type… (authored by flx).

Changed prior to commit:
  https://reviews.llvm.org/D98738?vs=335267=335632#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98738

Files:
  clang-tools-extra/clang-tidy/utils/Matchers.h
  clang-tools-extra/docs/clang-tidy/checks/performance-for-range-copy.rst
  
clang-tools-extra/docs/clang-tidy/checks/performance-unnecessary-copy-initialization.rst
  
clang-tools-extra/docs/clang-tidy/checks/performance-unnecessary-value-param.rst
  
clang-tools-extra/test/clang-tidy/checkers/performance-for-range-copy-allowed-types.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/performance-for-range-copy-allowed-types.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/performance-for-range-copy-allowed-types.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/performance-for-range-copy-allowed-types.cpp
@@ -1,5 +1,5 @@
 // RUN: %check_clang_tidy %s performance-for-range-copy %t -- \
-// RUN: -config="{CheckOptions: [{key: performance-for-range-copy.AllowedTypes, value: '[Pp]ointer$;[Pp]tr$;[Rr]ef(erence)?$'}]}" \
+// RUN: -config="{CheckOptions: [{key: performance-for-range-copy.AllowedTypes, value: '[Pp]ointer$;[Pp]tr$;[Rr]ef(erence)?$;qualified::Type;::fully::QualifiedType'}]}" \
 // RUN: -- -fno-delayed-template-parsing
 
 template 
@@ -63,6 +63,18 @@
 
 typedef SomeComplexTemplate NotTooComplexRef;
 
+namespace qualified {
+struct Type {
+  ~Type();
+};
+} // namespace qualified
+
+namespace fully {
+struct QualifiedType {
+  ~QualifiedType();
+};
+} // namespace fully
+
 void negativeSmartPointer() {
   for (auto P : View>()) {
 auto P2 = P;
@@ -124,3 +136,23 @@
 auto R2 = R;
   }
 }
+
+void negativeQualified() {
+  for (auto Q : View>()) {
+auto Q2 = Q;
+  }
+  using qualified::Type;
+  for (auto Q : View>()) {
+auto Q2 = Q;
+  }
+}
+
+void negativeFullyQualified() {
+  for (auto Q : View>()) {
+auto Q2 = Q;
+  }
+  using fully::QualifiedType;
+  for (auto Q : View>()) {
+auto Q2 = Q;
+  }
+}
Index: clang-tools-extra/docs/clang-tidy/checks/performance-unnecessary-value-param.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/performance-unnecessary-value-param.rst
+++ clang-tools-extra/docs/clang-tidy/checks/performance-unnecessary-value-param.rst
@@ -66,4 +66,7 @@
 
A semicolon-separated list of names of types allowed to be passed by value.
Regular expressions are accepted, e.g. `[Rr]ef(erence)?$` matches every type
-   with suffix `Ref`, `ref`, `Reference` and `reference`. The default is empty.
+   with suffix `Ref`, `ref`, `Reference` and `reference`. The default is
+   empty. If a name in the list contains the sequence `::` it is matched against
+   the qualified typename (i.e. `namespace::Type`, otherwise it is matched
+   against only the type name (i.e. `Type`).
Index: clang-tools-extra/docs/clang-tidy/checks/performance-unnecessary-copy-initialization.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/performance-unnecessary-copy-initialization.rst
+++ clang-tools-extra/docs/clang-tidy/checks/performance-unnecessary-copy-initialization.rst
@@ -43,5 +43,7 @@
 
A semicolon-separated list of names of types allowed to be initialized by
copying. Regular expressions are accepted, e.g. `[Rr]ef(erence)?$` matches
-   every type with suffix `Ref`, `ref`, `Reference` and `reference`. The
-   default is empty.
+   every type with suffix `Ref`, `ref`, `Reference` and `reference`. The default
+   is empty. If a name in the list contains the sequence `::` it is matched
+   against the qualified typename (i.e. `namespace::Type`, otherwise it is
+   matched against only the type name (i.e. `Type`).
Index: clang-tools-extra/docs/clang-tidy/checks/performance-for-range-copy.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/performance-for-range-copy.rst
+++ clang-tools-extra/docs/clang-tidy/checks/performance-for-range-copy.rst
@@ -31,4 +31,6 @@
A semicolon-separated list of names of types allowed to be copied in each
iteration. Regular expressions are accepted, e.g. `[Rr]ef(erence)?$` matches
every type with suffix `Ref`, `ref`, `Reference` and `reference`. The default
-   is empty.
+   is empty. If a name in the list contains the sequence `::` it is matched
+   against the qualified typename (i.e. `namespace::Type`, otherwise it is
+   matched against only the type name (i.e. `Type`).
Index: clang-tools-extra/clang-tidy/utils/Matchers.h

[clang-tools-extra] ddebed8 - [clang-tidy] performance-* checks: Match AllowedTypes against qualified type names when they contain "::".

[Felix Berger via cfe-commits]

Author: Felix Berger
Date: 2021-04-06T15:41:35-04:00
New Revision: ddebed8e9742add4372d54021cb55e06b655cfd6

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

LOG: [clang-tidy] performance-* checks: Match AllowedTypes against qualified 
type names when they contain "::".

This allows users to be more precise and exclude a type in a specific namespace
from triggering the check instead of excluding all types with the same
unqualified name.

This change should not interfere with correctly configured clang-tidy setups
since an AllowedType with "::" would never match.

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

Reviewed-by: ymandel, hokein

Added: 


Modified: 
clang-tools-extra/clang-tidy/utils/Matchers.h
clang-tools-extra/docs/clang-tidy/checks/performance-for-range-copy.rst

clang-tools-extra/docs/clang-tidy/checks/performance-unnecessary-copy-initialization.rst

clang-tools-extra/docs/clang-tidy/checks/performance-unnecessary-value-param.rst

clang-tools-extra/test/clang-tidy/checkers/performance-for-range-copy-allowed-types.cpp

Removed: 




diff  --git a/clang-tools-extra/clang-tidy/utils/Matchers.h 
b/clang-tools-extra/clang-tidy/utils/Matchers.h
index 9b765aedd13af..469ce4a9c4a4c 100644
--- a/clang-tools-extra/clang-tidy/utils/Matchers.h
+++ b/clang-tools-extra/clang-tidy/utils/Matchers.h
@@ -49,11 +49,76 @@ AST_MATCHER_FUNCTION(ast_matchers::TypeMatcher, 
isPointerToConst) {
   return pointerType(pointee(qualType(isConstQualified(;
 }
 
-AST_MATCHER_P(NamedDecl, matchesAnyListedName, std::vector,
-  NameList) {
-  return llvm::any_of(NameList, [](const std::string ) {
-  return llvm::Regex(Name).match(Node.getName());
+// A matcher implementation that matches a list of type name regular 
expressions
+// against a NamedDecl. If a regular expression contains the substring "::"
+// matching will occur against the qualified name, otherwise only the typename.
+class MatchesAnyListedNameMatcher
+: public ast_matchers::internal::MatcherInterface {
+public:
+  explicit MatchesAnyListedNameMatcher(llvm::ArrayRef NameList) {
+std::transform(
+NameList.begin(), NameList.end(), std::back_inserter(NameMatchers),
+[](const llvm::StringRef Name) { return NameMatcher(Name); });
+  }
+  bool matches(
+  const NamedDecl , ast_matchers::internal::ASTMatchFinder *Finder,
+  ast_matchers::internal::BoundNodesTreeBuilder *Builder) const override {
+return llvm::any_of(NameMatchers, [](const NameMatcher ) {
+  return NM.match(Node);
 });
+  }
+
+private:
+  class NameMatcher {
+llvm::Regex Regex;
+enum class MatchMode {
+  // Match against the unqualified name because the regular expression
+  // does not contain ":".
+  MatchUnqualified,
+  // Match against the qualified name because the regular expression
+  // contains ":" suggesting name and namespace should be matched.
+  MatchQualified,
+  // Match against the fully qualified name because the regular expression
+  // starts with ":".
+  MatchFullyQualified,
+};
+MatchMode Mode;
+
+  public:
+NameMatcher(const llvm::StringRef Regex)
+: Regex(Regex), Mode(determineMatchMode(Regex)) {}
+
+bool match(const NamedDecl ) const {
+  switch (Mode) {
+  case MatchMode::MatchQualified:
+return Regex.match(ND.getQualifiedNameAsString());
+  case MatchMode::MatchFullyQualified:
+return Regex.match("::" + ND.getQualifiedNameAsString());
+  default:
+return Regex.match(ND.getName());
+  }
+}
+
+  private:
+MatchMode determineMatchMode(llvm::StringRef Regex) {
+  if (Regex.startswith(":") || Regex.startswith("^:")) {
+return MatchMode::MatchFullyQualified;
+  }
+  return Regex.contains(":") ? MatchMode::MatchQualified
+ : MatchMode::MatchUnqualified;
+}
+  };
+
+  std::vector NameMatchers;
+};
+
+// Returns a matcher that matches NamedDecl's against a list of provided 
regular
+// expressions. If a regular expression contains starts ':' the NamedDecl's
+// qualified name will be used for matching, otherwise its name will be used.
+inline ::clang::ast_matchers::internal::Matcher
+matchesAnyListedName(llvm::ArrayRef NameList) {
+  return ::clang::ast_matchers::internal::makeMatcher(
+  new MatchesAnyListedNameMatcher(NameList));
 }
 
 } // namespace matchers

diff  --git 
a/clang-tools-extra/docs/clang-tidy/checks/performance-for-range-copy.rst 
b/clang-tools-extra/docs/clang-tidy/checks/performance-for-range-copy.rst
index a48d6d4cb5399..01fde9580e2a0 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/performance-for-range-copy.rst
+++ 

[PATCH] D99973: [Windows] Add test coverage for line endings when rewriting includes

[Alexandre Ganea via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
aganea marked an inline comment as done.
Closed by commit rG8fbc05acd553: [Windows] Add test coverage for line endings 
when rewriting includes (authored by aganea).

Changed prior to commit:
  https://reviews.llvm.org/D99973?vs=335574=335630#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99973

Files:
  clang/test/Frontend/rewrite-includes-macros.cpp


Index: clang/test/Frontend/rewrite-includes-macros.cpp
===
--- /dev/null
+++ clang/test/Frontend/rewrite-includes-macros.cpp
@@ -0,0 +1,14 @@
+// REQUIRES: system-windows
+// RUN: %clang_cl /E -Xclang -frewrite-includes %s | %clang_cl /c -Xclang 
-verify /Tp -
+// expected-no-diagnostics
+
+int foo();
+int bar();
+#define HELLO \
+  foo(); \
+  bar();
+
+int main() {
+  HELLO
+  return 0;
+}


Index: clang/test/Frontend/rewrite-includes-macros.cpp
===
--- /dev/null
+++ clang/test/Frontend/rewrite-includes-macros.cpp
@@ -0,0 +1,14 @@
+// REQUIRES: system-windows
+// RUN: %clang_cl /E -Xclang -frewrite-includes %s | %clang_cl /c -Xclang -verify /Tp -
+// expected-no-diagnostics
+
+int foo();
+int bar();
+#define HELLO \
+  foo(); \
+  bar();
+
+int main() {
+  HELLO
+  return 0;
+}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] 8fbc05a - [Windows] Add test coverage for line endings when rewriting includes

[Alexandre Ganea via cfe-commits]

Author: Alexandre Ganea
Date: 2021-04-06T15:38:19-04:00
New Revision: 8fbc05acd5531a8bb74f689699e8de2788bcb769

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

LOG: [Windows] Add test coverage for line endings when rewriting includes

Validate that we're properly generating a single line ending on Windows when
using -frewrite-includes. Otherwise we're breaking split-line macros. The test
fails before 23929af383f27a6ddf23704192a25591481152b3.

See discussion in https://reviews.llvm.org/D96363#2650460 and D99426

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

Added: 
clang/test/Frontend/rewrite-includes-macros.cpp

Modified: 


Removed: 




diff  --git a/clang/test/Frontend/rewrite-includes-macros.cpp 
b/clang/test/Frontend/rewrite-includes-macros.cpp
new file mode 100644
index 0..4a2de546d4554
--- /dev/null
+++ b/clang/test/Frontend/rewrite-includes-macros.cpp
@@ -0,0 +1,14 @@
+// REQUIRES: system-windows
+// RUN: %clang_cl /E -Xclang -frewrite-includes %s | %clang_cl /c -Xclang 
-verify /Tp -
+// expected-no-diagnostics
+
+int foo();
+int bar();
+#define HELLO \
+  foo(); \
+  bar();
+
+int main() {
+  HELLO
+  return 0;
+}



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

[PATCH] D99973: [Windows] Add test coverage for line endings when rewriting includes

[Alexandre Ganea via Phabricator via cfe-commits]

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



Comment at: clang/test/Frontend/rewrite-includes-macros.cpp:15
+}
\ No newline at end of file


mstorsjo wrote:
> I guess the missing newline at end of file isn't one of the aspects that 
> needs to be tested?
It is not indeed, I'll fix that, thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99973

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

[PATCH] D99447: [OpenMP] Define omp_is_initial_device() variants in omp.h

[Jon Chesterfield via Phabricator via cfe-commits]

JonChesterfield accepted this revision.
JonChesterfield added a comment.

LGTM, thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99447

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

[PATCH] D99973: [Windows] Add test coverage for line endings when rewriting includes

[Abhina Sree via Phabricator via cfe-commits]

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

LGTM, thanks for adding a testcase!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99973

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

[PATCH] D99893: [WIP] Replace std::forward & std::move by cast expressions during Sema

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

Our builtin logic is capable of recognizing user declarations as builtins 
without any sort of explicit annotation in source.  That's how we recognize C 
library builtins, for example.  As Richard points out, that logic isn't really 
set up to do the right thing for namespaced declarations (or template ones, for 
that matter), but that seems fixable.

The namespace issue might be annoying because of the inline namespaces that 
some standard libraries used for ABI versioning.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99893

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

[PATCH] D99984: [RISCV] Prevent __builtin_riscv_orc_b_64 from being compiled RV32 target.

[Craig Topper via Phabricator via cfe-commits]

craig.topper created this revision.
craig.topper added reviewers: asb, frasercrmck, luismarques, jrtc27, evandro, 
HsiangKai, khchen, arcbbb.
Herald added subscribers: StephenFan, vkmr, apazos, sameer.abuasal, s.egerton, 
Jim, benna, psnobl, jocewei, PkmX, the_o, brucehoult, MartinMosbeck, rogfer01, 
edward-jones, zzheng, shiva0217, kito-cheng, niosHD, sabuasal, simoncook, 
johnrusso, rbar.
craig.topper requested review of this revision.
Herald added a subscriber: MaskRay.
Herald added a project: clang.

The backend can't handle this and will throw a fatal error from
type legalization. It's easy enough to fix that for this intrinsic
by just splitting the IR intrinsic since it works on individual bytes.

There will be other intrinsics in the future that would be harder
to support through splitting, for example grev, gorc, and shfl. Those
would require a compare and a select be inserted to check the MSB of
their control input.

This patch adds support for preventing this in the frontend with
a nice diagnostic.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D99984

Files:
  clang/include/clang/Basic/BuiltinsRISCV.def
  clang/lib/Basic/Targets/RISCV.cpp
  clang/lib/Basic/Targets/RISCV.h
  clang/lib/Sema/SemaChecking.cpp
  clang/test/CodeGen/RISCV/rvb-intrinsics/riscv32-zbb-error.c


Index: clang/test/CodeGen/RISCV/rvb-intrinsics/riscv32-zbb-error.c
===
--- /dev/null
+++ clang/test/CodeGen/RISCV/rvb-intrinsics/riscv32-zbb-error.c
@@ -0,0 +1,6 @@
+// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py
+// RUN: %clang_cc1 -triple riscv32 -target-feature +experimental-zbb -verify 
%s -o -
+
+int orc_b_64(int a) {
+  return __builtin_riscv_orc_b_64(a); // expected-error {{builtin requires 
'RV64' extension support to be enabled}}
+}
Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -3422,12 +3422,18 @@
   Features.split(ReqFeatures, ',');
 
   // Check if each required feature is included
-  for (auto  : ReqFeatures) {
-if (TI.hasFeature(I))
+  for (StringRef F : ReqFeatures) {
+if (TI.hasFeature(F))
   continue;
+
+// If the feature is 64bit, alter the string so it will print better in
+// the diagnostic.
+if (F == "64bit")
+  F = "RV64";
+
 // Convert features like "zbr" and "experimental-zbr" to "Zbr".
-I.consume_front("experimental-");
-std::string FeatureStr = I.str();
+F.consume_front("experimental-");
+std::string FeatureStr = F.str();
 FeatureStr[0] = std::toupper(FeatureStr[0]);
 
 // Error message
Index: clang/lib/Basic/Targets/RISCV.h
===
--- clang/lib/Basic/Targets/RISCV.h
+++ clang/lib/Basic/Targets/RISCV.h
@@ -99,6 +99,11 @@
 
   std::string convertConstraint(const char *) const override;
 
+  bool
+  initFeatureMap(llvm::StringMap , DiagnosticsEngine ,
+ StringRef CPU,
+ const std::vector ) const override;
+
   bool hasFeature(StringRef Feature) const override;
 
   bool handleTargetFeatures(std::vector ,
Index: clang/lib/Basic/Targets/RISCV.cpp
===
--- clang/lib/Basic/Targets/RISCV.cpp
+++ clang/lib/Basic/Targets/RISCV.cpp
@@ -239,6 +239,16 @@
  Builtin::FirstTSBuiltin);
 }
 
+bool RISCVTargetInfo::initFeatureMap(
+llvm::StringMap , DiagnosticsEngine , StringRef CPU,
+const std::vector ) const {
+
+  if (getTriple().getArch() == llvm::Triple::riscv64)
+Features["64bit"] = true;
+
+  return TargetInfo::initFeatureMap(Features, Diags, CPU, FeaturesVec);
+}
+
 /// Return true if has this feature, need to sync with handleTargetFeatures.
 bool RISCVTargetInfo::hasFeature(StringRef Feature) const {
   bool Is64Bit = getTriple().getArch() == llvm::Triple::riscv64;
@@ -246,6 +256,7 @@
   .Case("riscv", true)
   .Case("riscv32", !Is64Bit)
   .Case("riscv64", Is64Bit)
+  .Case("64bit", Is64Bit)
   .Case("m", HasM)
   .Case("a", HasA)
   .Case("f", HasF)
Index: clang/include/clang/Basic/BuiltinsRISCV.def
===
--- clang/include/clang/Basic/BuiltinsRISCV.def
+++ clang/include/clang/Basic/BuiltinsRISCV.def
@@ -19,7 +19,7 @@
 
 // Zbb extension
 TARGET_BUILTIN(__builtin_riscv_orc_b_32, "ZiZi", "nc", "experimental-zbb")
-TARGET_BUILTIN(__builtin_riscv_orc_b_64, "WiWi", "nc", "experimental-zbb")
+TARGET_BUILTIN(__builtin_riscv_orc_b_64, "WiWi", "nc", 
"experimental-zbb,64bit")
 
 // Zbc extension
 TARGET_BUILTIN(__builtin_riscv_clmul, "LiLiLi", "nc", "experimental-zbc")


Index: clang/test/CodeGen/RISCV/rvb-intrinsics/riscv32-zbb-error.c
===
--- /dev/null
+++ 

[PATCH] D99484: Use `GNUInstallDirs` to support custom installation dirs.

[Louis Dionne via Phabricator via cfe-commits]

ldionne accepted this revision as: libc++, libc++abi.
ldionne added a comment.

LGTM for libcxx and libcxxabi.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99484

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

[PATCH] D99973: [Windows] Add test coverage for line endings when rewriting includes

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo added a comment.

Not very familiar with the feature, but from the linked discussion it looks 
sensible to me, so +1, I can give a formal stamp a bit later if nobody else 
does.




Comment at: clang/test/Frontend/rewrite-includes-macros.cpp:15
+}
\ No newline at end of file


I guess the missing newline at end of file isn't one of the aspects that needs 
to be tested?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99973

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

[PATCH] D99898: [clang, test] Fix use of undef FileCheck var

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

`NUW_RN` isn't a good name for the variable in this alternative since it's not 
`readnone` anymore.  With an appropriate rename, LGTM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99898

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

[PATCH] D99225: [clang] tests: cleanup, update and add some new ones

[Nico Weber via Phabricator via cfe-commits]

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

This looks fine to me. Maybe wait for a day or two to see if richardsmith wants 
to say anything, else this is good to go.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99225

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

[PATCH] D98193: [CUDA][HIP] Allow non-ODR use of host var in device

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added inline comments.



Comment at: clang/lib/Sema/SemaExpr.cpp:17050
+auto Target = SemaRef.IdentifyCUDATarget(FD);
+if (Var && Var->isFileVarDecl() && !Var->hasAttr() &&
+!Var->hasAttr() && !Var->hasAttr() &&

rsmith wrote:
> I suspect you want `hasGlobalStorage` rather than `isFileVarDecl` here (that 
> is, preserve the condition from the deleted code), in order to disallow use 
> of host-side local static variables from device-side functions:
> 
> ```
> __host__ void f() {
>   static int n;
>   struct X {
> __device__ void g() { ++n; }
>   };
>   // ...
> }
> ```
For function scope static variable, if the smallest enclosing function is 
device or device host function, the static variable without `__device__` or 
`__constant__` attribute is allowed and the variable is emitted at device side 
(https://godbolt.org/z/PY5d3WGas). In that case, a device function is allowed 
to access that static variable even if it does not have `__device__` or 
`__constant__` attribute.

I will make changes to handle the function scope static variable.


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

https://reviews.llvm.org/D98193

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

[PATCH] D99898: [clang, test] Fix use of undef FileCheck var

[Thomas Preud'homme via Phabricator via cfe-commits]

thopre updated this revision to Diff 335618.
thopre added a comment.

Change attribute check to a positive match


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99898

Files:
  clang/test/CodeGen/libcalls.c


Index: clang/test/CodeGen/libcalls.c
===
--- clang/test/CodeGen/libcalls.c
+++ clang/test/CodeGen/libcalls.c
@@ -88,9 +88,9 @@
 // CHECK-NO: declare double @atan(double) [[NUW_RN:#[0-9]+]]
 // CHECK-NO: declare x86_fp80 @atanl(x86_fp80) [[NUW_RN]]
 // CHECK-NO: declare float @atanf(float) [[NUW_RN]]
-// CHECK-YES-NOT: declare double @atan(double) [[NUW_RN]]
-// CHECK-YES-NOT: declare x86_fp80 @atanl(x86_fp80) [[NUW_RN]]
-// CHECK-YES-NOT: declare float @atanf(float) [[NUW_RN]]
+// CHECK-YES: declare double @atan(double) [[NUW_RN:#[0-9]+]]
+// CHECK-YES: declare x86_fp80 @atanl(x86_fp80) [[NUW_RN]]
+// CHECK-YES: declare float @atanf(float) [[NUW_RN]]
 
   double atan2_ = atan2(d, 2);
   long double atan2l_ = atan2l(ld, ld);
@@ -98,9 +98,9 @@
 // CHECK-NO: declare double @atan2(double, double) [[NUW_RN]]
 // CHECK-NO: declare x86_fp80 @atan2l(x86_fp80, x86_fp80) [[NUW_RN]]
 // CHECK-NO: declare float @atan2f(float, float) [[NUW_RN]]
-// CHECK-YES-NOT: declare double @atan2(double, double) [[NUW_RN]]
-// CHECK-YES-NOT: declare x86_fp80 @atan2l(x86_fp80, x86_fp80) [[NUW_RN]]
-// CHECK-YES-NOT: declare float @atan2f(float, float) [[NUW_RN]]
+// CHECK-YES: declare double @atan2(double, double) [[NUW_RN]]
+// CHECK-YES: declare x86_fp80 @atan2l(x86_fp80, x86_fp80) [[NUW_RN]]
+// CHECK-YES: declare float @atan2f(float, float) [[NUW_RN]]
 
   double exp_ = exp(d);
   long double expl_ = expl(ld);
@@ -108,9 +108,9 @@
 // CHECK-NO: declare double @llvm.exp.f64(double) [[NUW_RNI]]
 // CHECK-NO: declare x86_fp80 @llvm.exp.f80(x86_fp80) [[NUW_RNI]]
 // CHECK-NO: declare float @llvm.exp.f32(float) [[NUW_RNI]]
-// CHECK-YES-NOT: declare double @exp(double) [[NUW_RN]]
-// CHECK-YES-NOT: declare x86_fp80 @expl(x86_fp80) [[NUW_RN]]
-// CHECK-YES-NOT: declare float @expf(float) [[NUW_RN]]
+// CHECK-YES: declare double @exp(double) [[NUW_RN]]
+// CHECK-YES: declare x86_fp80 @expl(x86_fp80) [[NUW_RN]]
+// CHECK-YES: declare float @expf(float) [[NUW_RN]]
 
   double log_ = log(d);
   long double logl_ = logl(ld);
@@ -118,10 +118,11 @@
 // CHECK-NO: declare double @llvm.log.f64(double) [[NUW_RNI]]
 // CHECK-NO: declare x86_fp80 @llvm.log.f80(x86_fp80) [[NUW_RNI]]
 // CHECK-NO: declare float @llvm.log.f32(float) [[NUW_RNI]]
-// CHECK-YES-NOT: declare double @log(double) [[NUW_RN]]
-// CHECK-YES-NOT: declare x86_fp80 @logl(x86_fp80) [[NUW_RN]]
-// CHECK-YES-NOT: declare float @logf(float) [[NUW_RN]]
+// CHECK-YES: declare double @log(double) [[NUW_RN]]
+// CHECK-YES: declare x86_fp80 @logl(x86_fp80) [[NUW_RN]]
+// CHECK-YES: declare float @logf(float) [[NUW_RN]]
 }
 
+// CHECK-YES: attributes [[NUW_RN]] = { nounwind "frame-pointer"="none" 
"no-trapping-math"="true" "stack-protector-buffer-size"="8" 
"target-features"="+cx8,+x87" }
 // CHECK-NO-DAG: attributes [[NUW_RN]] = { nounwind readnone{{.*}} }
 // CHECK-NO-DAG: attributes [[NUW_RNI]] = { nofree nosync nounwind readnone 
speculatable willreturn }


Index: clang/test/CodeGen/libcalls.c
===
--- clang/test/CodeGen/libcalls.c
+++ clang/test/CodeGen/libcalls.c
@@ -88,9 +88,9 @@
 // CHECK-NO: declare double @atan(double) [[NUW_RN:#[0-9]+]]
 // CHECK-NO: declare x86_fp80 @atanl(x86_fp80) [[NUW_RN]]
 // CHECK-NO: declare float @atanf(float) [[NUW_RN]]
-// CHECK-YES-NOT: declare double @atan(double) [[NUW_RN]]
-// CHECK-YES-NOT: declare x86_fp80 @atanl(x86_fp80) [[NUW_RN]]
-// CHECK-YES-NOT: declare float @atanf(float) [[NUW_RN]]
+// CHECK-YES: declare double @atan(double) [[NUW_RN:#[0-9]+]]
+// CHECK-YES: declare x86_fp80 @atanl(x86_fp80) [[NUW_RN]]
+// CHECK-YES: declare float @atanf(float) [[NUW_RN]]
 
   double atan2_ = atan2(d, 2);
   long double atan2l_ = atan2l(ld, ld);
@@ -98,9 +98,9 @@
 // CHECK-NO: declare double @atan2(double, double) [[NUW_RN]]
 // CHECK-NO: declare x86_fp80 @atan2l(x86_fp80, x86_fp80) [[NUW_RN]]
 // CHECK-NO: declare float @atan2f(float, float) [[NUW_RN]]
-// CHECK-YES-NOT: declare double @atan2(double, double) [[NUW_RN]]
-// CHECK-YES-NOT: declare x86_fp80 @atan2l(x86_fp80, x86_fp80) [[NUW_RN]]
-// CHECK-YES-NOT: declare float @atan2f(float, float) [[NUW_RN]]
+// CHECK-YES: declare double @atan2(double, double) [[NUW_RN]]
+// CHECK-YES: declare x86_fp80 @atan2l(x86_fp80, x86_fp80) [[NUW_RN]]
+// CHECK-YES: declare float @atan2f(float, float) [[NUW_RN]]
 
   double exp_ = exp(d);
   long double expl_ = expl(ld);
@@ -108,9 +108,9 @@
 // CHECK-NO: declare double @llvm.exp.f64(double) [[NUW_RNI]]
 // CHECK-NO: declare x86_fp80 @llvm.exp.f80(x86_fp80) [[NUW_RNI]]
 // CHECK-NO: declare float @llvm.exp.f32(float) [[NUW_RNI]]
-// CHECK-YES-NOT: 

[PATCH] D82317: [Clang/Test]: Update tests where `noundef` attribute is necessary

[Gui Andrade via Phabricator via cfe-commits]

guiand added a comment.

Sorry, I must've missed this. @aqjune, if you'd be willing to take on this 
change, that would be amazing (I haven't got much time to update these patches 
lately).

We can make these changes incrementally, too; say, one test subfolder at a 
time. Each patch could add the noundef enable option, and then once all the 
tests are ready we can batch remove the flag.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82317

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

[PATCH] D99983: Provide TreeTransform::TransformAttr the transformed statement; NFC

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman created this revision.
aaron.ballman added reviewers: rsmith, haberman.
aaron.ballman requested review of this revision.
Herald added a project: clang.

It is useful statement an attribute is being applied to when performing 
semantic processing of the attribute during template instantiation. This 
functionality is not currently needed by existing attributes, but is 
anticipated to be used by new attributes being worked on.

One design choice with this is whether to transform attributes then the 
attributed statement, or to transform the attributed statement and then the 
attributes. I elected to transform the attributed statement first because it 
seems less likely that the statement transformation would require the 
instanatiated attributes compared to when we transform attributes (those would 
benefit from knowing the fully instantiated statement). This patch passes a 
non-`const Stmt *` to `TransformAttr()` in case the attribute instantiation 
needs to modify the attributed statement for some reason.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D99983

Files:
  clang/lib/Sema/SemaTemplateInstantiate.cpp
  clang/lib/Sema/TreeTransform.h

Index: clang/lib/Sema/TreeTransform.h
===
--- clang/lib/Sema/TreeTransform.h
+++ clang/lib/Sema/TreeTransform.h
@@ -379,8 +379,14 @@
   /// of attribute. Subclasses may override this function to transform
   /// attributed statements using some other mechanism.
   ///
+  /// The \c TransformedStmt parameter points to the substituted statement and
+  /// is non-const explicitly so that transformations of the attribute that
+  /// need to set state on the statement they apply to can do so if needed.
+  /// Note that \c TransformedStmt will be null when transforming type
+  /// attributes.
+  ///
   /// \returns the transformed attribute
-  const Attr *TransformAttr(const Attr *S);
+  const Attr *TransformAttr(const Attr *A, Stmt *TransformedStmt);
 
 /// Transform the specified attribute.
 ///
@@ -390,7 +396,9 @@
 /// \returns the transformed attribute.
 #define ATTR(X)
 #define PRAGMA_SPELLING_ATTR(X)\
-  const X##Attr *Transform##X##Attr(const X##Attr *R) { return R; }
+  const X##Attr *Transform##X##Attr(const X##Attr *R, Stmt *TransformedStmt) { \
+return R;  \
+  }
 #include "clang/Basic/AttrList.inc"
 
   /// Transform the given expression.
@@ -6734,7 +6742,8 @@
 
   // oldAttr can be null if we started with a QualType rather than a TypeLoc.
   const Attr *oldAttr = TL.getAttr();
-  const Attr *newAttr = oldAttr ? getDerived().TransformAttr(oldAttr) : nullptr;
+  const Attr *newAttr =
+  oldAttr ? getDerived().TransformAttr(oldAttr, nullptr) : nullptr;
   if (oldAttr && !newAttr)
 return QualType();
 
@@ -7287,19 +7296,20 @@
 }
 
 template 
-const Attr *TreeTransform::TransformAttr(const Attr *R) {
-  if (!R)
-return R;
+const Attr *TreeTransform::TransformAttr(const Attr *A,
+  Stmt *TransformedStmt) {
+  if (!A)
+return A;
 
-  switch (R->getKind()) {
+  switch (A->getKind()) {
 // Transform attributes with a pragma spelling by calling TransformXXXAttr.
 #define ATTR(X)
 #define PRAGMA_SPELLING_ATTR(X)\
   case attr::X:\
-return getDerived().Transform##X##Attr(cast(R));
+return getDerived().Transform##X##Attr(cast(A), TransformedStmt);
 #include "clang/Basic/AttrList.inc"
   default:
-return R;
+return A;
   }
 }
 
@@ -7307,20 +7317,26 @@
 StmtResult
 TreeTransform::TransformAttributedStmt(AttributedStmt *S,
 StmtDiscardKind SDK) {
+  // Transform the attributed statement first so that we can pass the
+  // transformed version in to the attribute handler. This is necessary because
+  // attributes that need to perform semantic checking are more likely to need
+  // the transformed statement than statement semantic checking is likely to
+  // need the transformed attributes. In such a case, the TransformFooAttr()
+  // function can mutate the non-const transformed statement that it is passed.
+  StmtResult SubStmt = getDerived().TransformStmt(S->getSubStmt(), SDK);
+  if (SubStmt.isInvalid())
+return StmtError();
+
   bool AttrsChanged = false;
   SmallVector Attrs;
 
   // Visit attributes and keep track if any are transformed.
   for (const auto *I : S->getAttrs()) {
-const Attr *R = getDerived().TransformAttr(I);
+const Attr *R = getDerived().TransformAttr(I, SubStmt.get());
 AttrsChanged |= (I != R);
 Attrs.push_back(R);
   }
 
-  StmtResult SubStmt = getDerived().TransformStmt(S->getSubStmt(), SDK);
-  if (SubStmt.isInvalid())
-return StmtError();
-
   if (SubStmt.get() == S->getSubStmt() 

[PATCH] D99037: [Matrix] Implement C-style explicit type conversions for matrix types

[Saurabh Jha via Phabricator via cfe-commits]

SaurabhJha added inline comments.



Comment at: clang/include/clang/AST/OperationKinds.def:185
+/// CK_MatrixCast - A cast between matrix types of the same dimensions.
+CAST_OPERATION(MatrixCast)
+

This line is causing me issues that I don't know how to solve. If we leave it 
in the current place on line 185. The new tests Sema and CodeGen tests are 
passing but Clang.SemaOpenCL::sampler_t.cl and 
Clang.SemaOpenCL::sampler_t_overload.cl (along with other sampler tests) are 
failing.

And if I move it to the bottom, right after `CAST_OPERATION(IntToOCLSampler)`, 
the sampler tests pass but the new matrix conversion tests are failing. Am I 
missing something here? My understanding was implementing new cast kind is just 
the matter of defining a `CAST_OPERATION` in this file.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99037

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

[PATCH] D71726: Let clang atomic builtins fetch add/sub support floating point types

[John McCall via Phabricator via cfe-commits]

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

Thanks, LGTM


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

https://reviews.llvm.org/D71726

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

[PATCH] D99675: RFC [llvm][clang] Create new intrinsic llvm.arith.fence to control FP optimization at expression level

[Melanie Blower via Phabricator via cfe-commits]

mibintc added a comment.

In D99675#2671924 , @efriedma wrote:

>> The expression “llvm.arith.fence(a * b) + c” means that “a * b” must happen 
>> before “+ c” and FMA guarantees that, but to prevent later optimizations 
>> from unpacking the FMA the correct transformation needs to be:
>>
>> llvm.arith.fence(a * b) + c  →  llvm.arith.fence(FMA(a, b, c))
>
> Does this actually block later transforms from unpacking the FMA?  Maybe if 
> the FMA isn't marked "fast"...

I'd like @pengfei to reply to this question. I think the overall idea is that 
many of the optimizations are pattern based, and the existing pattern wouldn't 
match the new intrinsic.

> 
>
> How is llvm.arith.fence() different from using "freeze" on a floating-point 
> value?  The goal isn't really the same, sure, but the effects seem similar at 
> first glance.

Initially we thought the intrinsic "ssa.copy" could serve. However ssa.copy is 
for a different purpose and it gets optimized away.  We want arith.fence to 
survive through codegen, that's one reason why we think a new intrinsic is 
needed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99675

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

[PATCH] D99037: [Matrix] Implement C-style explicit type conversions for matrix types

[Saurabh Jha via Phabricator via cfe-commits]

SaurabhJha added inline comments.



Comment at: clang/lib/CodeGen/CGExprScalar.cpp:1324
+
+if (SrcElementTy->isFloatTy() || SrcElementTy->isDoubleTy()) {
+  QualType DstElementType = 
DstType->castAs()->getElementType();

fhahn wrote:
> SaurabhJha wrote:
> > fhahn wrote:
> > > I think we should support all floating point type here 
> > > (`isFloatingPointTy`). Basically we want the same rules as for integer 
> > > types (around line 1418). Perhaps it would be possible to generalize this 
> > > code in a separate function that takes the LLVM types & the element 
> > > type/scalar types?
> > I think we should move this code to `MatrixBuilder` but then generalising 
> > floating point after that would be problematic since the code is splitted 
> > across this function and in MatrixBuilder, right?
> That's a good point. Sharing the logic with the scalar case in Clang directly 
> seems much more valuable than having it in the MatrixBuilder IMO.
Whoops, I just moved the code to MatrixBuilder. Will revert it back and 
generalise it :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99037

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

[PATCH] D99675: RFC [llvm][clang] Create new intrinsic llvm.arith.fence to control FP optimization at expression level

[Kevin B. Smith via Phabricator via cfe-commits]

kbsmith1 added a comment.

In D99675#2671924 , @efriedma wrote:

>> The expression “llvm.arith.fence(a * b) + c” means that “a * b” must happen 
>> before “+ c” and FMA guarantees that, but to prevent later optimizations 
>> from unpacking the FMA the correct transformation needs to be:
>>
>> llvm.arith.fence(a * b) + c  →  llvm.arith.fence(FMA(a, b, c))
>
> Does this actually block later transforms from unpacking the FMA?  Maybe if 
> the FMA isn't marked "fast"...

I think we could define llvm.arith.fence to be such that this FMA contraction 
isn't legal/correct, or it could be left as is.  In the implementation that was 
used for the Intel compiler FMA contraction did not occur across an an __fence 
boundary.  It is unclear whether that was intended as the semantic, or if we 
just never bothered to implement that contraction.
Not allowing the FMA contraction across the llvm.arith.fence would make 
unpacking an FMA allowed under the same circumstances that LLVM currently 
allows that.

> 
>
> How is llvm.arith.fence() different from using "freeze" on a floating-point 
> value?  The goal isn't really the same, sure, but the effects seem similar at 
> first glance.

They are similar.  However, fence is a no-op if the operand can be proven not 
to be undef or poison, and in such circumstances could be removed by an 
optimizer.  llvm.arith.fence cannot be removed by an optimizer, because doing 
so might allow instructions that were "outside" the fence from being 
reassociated/distrbuted with the instructions/operands that were inside the 
fence.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99675

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

[PATCH] D99194: [analyzer] Fix body farm for Obj-C++ properties

[Valeriy Savchenko via Phabricator via cfe-commits]

vsavchenko updated this revision to Diff 335615.
vsavchenko added a comment.

Set Prop and IVar at the same time


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99194

Files:
  clang/lib/Analysis/BodyFarm.cpp
  clang/test/Analysis/properties.mm

Index: clang/test/Analysis/properties.mm
===
--- clang/test/Analysis/properties.mm
+++ clang/test/Analysis/properties.mm
@@ -77,3 +77,23 @@
 
   clang_analyzer_eval(w.inner.value == 42); // expected-warning{{TRUE}}
 }
+
+@protocol NoDirectPropertyDecl
+@property IntWrapperStruct inner;
+@end
+@interface NoDirectPropertyDecl 
+@end
+@implementation NoDirectPropertyDecl
+@synthesize inner;
+@end
+
+// rdar://67416721
+void testNoDirectPropertyDecl(NoDirectPropertyDecl *w) {
+  clang_analyzer_eval(w.inner.value == w.inner.value); // expected-warning{{TRUE}}
+
+  int origValue = w.inner.value;
+  if (origValue != 42)
+return;
+
+  clang_analyzer_eval(w.inner.value == 42); // expected-warning{{TRUE}}
+}
Index: clang/lib/Analysis/BodyFarm.cpp
===
--- clang/lib/Analysis/BodyFarm.cpp
+++ clang/lib/Analysis/BodyFarm.cpp
@@ -742,8 +742,9 @@
 
 static Stmt *createObjCPropertyGetter(ASTContext ,
   const ObjCMethodDecl *MD) {
-// First, find the backing ivar.
+  // First, find the backing ivar.
   const ObjCIvarDecl *IVar = nullptr;
+  const ObjCPropertyDecl *Prop = nullptr;
 
   // Property accessor stubs sometimes do not correspond to any property decl
   // in the current interface (but in a superclass). They still have a
@@ -751,54 +752,57 @@
   if (MD->isSynthesizedAccessorStub()) {
 const ObjCInterfaceDecl *IntD = MD->getClassInterface();
 const ObjCImplementationDecl *ImpD = IntD->getImplementation();
-for (const auto *PI: ImpD->property_impls()) {
-  if (const ObjCPropertyDecl *P = PI->getPropertyDecl()) {
-if (P->getGetterName() == MD->getSelector())
-  IVar = P->getPropertyIvarDecl();
+for (const auto *PI : ImpD->property_impls()) {
+  if (const ObjCPropertyDecl *Candidate = PI->getPropertyDecl()) {
+if (Candidate->getGetterName() == MD->getSelector()) {
+  Prop = Candidate;
+  IVar = Prop->getPropertyIvarDecl();
+}
   }
 }
   }
 
   if (!IVar) {
-const ObjCPropertyDecl *Prop = MD->findPropertyDecl();
+Prop = MD->findPropertyDecl();
 IVar = findBackingIvar(Prop);
-if (!IVar)
-  return nullptr;
+  }
 
-// Ignore weak variables, which have special behavior.
-if (Prop->getPropertyAttributes() & ObjCPropertyAttribute::kind_weak)
-  return nullptr;
+  if (!IVar || !Prop)
+return nullptr;
+
+  // Ignore weak variables, which have special behavior.
+  if (Prop->getPropertyAttributes() & ObjCPropertyAttribute::kind_weak)
+return nullptr;
 
-// Look to see if Sema has synthesized a body for us. This happens in
-// Objective-C++ because the return value may be a C++ class type with a
-// non-trivial copy constructor. We can only do this if we can find the
-// @synthesize for this property, though (or if we know it's been auto-
-// synthesized).
-const ObjCImplementationDecl *ImplDecl =
+  // Look to see if Sema has synthesized a body for us. This happens in
+  // Objective-C++ because the return value may be a C++ class type with a
+  // non-trivial copy constructor. We can only do this if we can find the
+  // @synthesize for this property, though (or if we know it's been auto-
+  // synthesized).
+  const ObjCImplementationDecl *ImplDecl =
   IVar->getContainingInterface()->getImplementation();
-if (ImplDecl) {
-  for (const auto *I : ImplDecl->property_impls()) {
-if (I->getPropertyDecl() != Prop)
-  continue;
-
-if (I->getGetterCXXConstructor()) {
-  ASTMaker M(Ctx);
-  return M.makeReturn(I->getGetterCXXConstructor());
-}
+  if (ImplDecl) {
+for (const auto *I : ImplDecl->property_impls()) {
+  if (I->getPropertyDecl() != Prop)
+continue;
+
+  if (I->getGetterCXXConstructor()) {
+ASTMaker M(Ctx);
+return M.makeReturn(I->getGetterCXXConstructor());
   }
 }
-
-// Sanity check that the property is the same type as the ivar, or a
-// reference to it, and that it is either an object pointer or trivially
-// copyable.
-if (!Ctx.hasSameUnqualifiedType(IVar->getType(),
-Prop->getType().getNonReferenceType()))
-  return nullptr;
-if (!IVar->getType()->isObjCLifetimeType() &&
-!IVar->getType().isTriviallyCopyableType(Ctx))
-  return nullptr;
   }
 
+  // Sanity check that the property is the same type as the ivar, or a
+  // reference to it, and that it is either an object pointer or trivially
+  // copyable.
+  if 

[PATCH] D99037: [Matrix] Implement C-style explicit type conversions for matrix types

[Florian Hahn via Phabricator via cfe-commits]

fhahn added inline comments.



Comment at: clang/lib/CodeGen/CGExprScalar.cpp:1324
+
+if (SrcElementTy->isFloatTy() || SrcElementTy->isDoubleTy()) {
+  QualType DstElementType = 
DstType->castAs()->getElementType();

SaurabhJha wrote:
> fhahn wrote:
> > I think we should support all floating point type here 
> > (`isFloatingPointTy`). Basically we want the same rules as for integer 
> > types (around line 1418). Perhaps it would be possible to generalize this 
> > code in a separate function that takes the LLVM types & the element 
> > type/scalar types?
> I think we should move this code to `MatrixBuilder` but then generalising 
> floating point after that would be problematic since the code is splitted 
> across this function and in MatrixBuilder, right?
That's a good point. Sharing the logic with the scalar case in Clang directly 
seems much more valuable than having it in the MatrixBuilder IMO.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99037

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

[PATCH] D99225: [clang] tests: cleanup, update and add some new ones

[Arthur O'Dwyer via Phabricator via cfe-commits]

Quuxplusone added a comment.

There's unlikely to be any further substantive comments from me, and this 
basically LGTM (or, in the places it looks bad, it's just the pre-existing 
awfulness of how the Clang test suite is organized).
I think you need to either get someone's attention to approve this, or approve 
it yourself. :)




Comment at: clang/test/SemaCXX/constant-expression-cxx11.cpp:393-406
 // Proposed DR: copy-elision doesn't trigger lifetime extension.
-// expected-warning@+1 2{{temporary whose address is used as value of local 
variable 'b5' will be destroyed at the end of the full-expression}}
+// cxx11-warning@+1 2{{temporary whose address is used as value of local 
variable 'b5' will be destroyed at the end of the full-expression}}
 constexpr B b5 = B{ {0}, {0} }; // expected-error {{constant expression}} 
expected-note {{reference to temporary}} expected-note {{here}}
 
 namespace NestedNonStatic {
   // Proposed DR: for a reference constant expression to refer to a static
   // storage duration temporary, that temporary must itself be initialized

I wonder if you ought to just leave this test alone. It's got `cxx11` in the 
name of the test, and it does a lot of things that are deprecated in 20/2b 
which you're having to work around.
No strong opinion, but mainly because I haven't looked closely.
If your upcoming NRVO patches actually have some effect on constexpr 
evaluation, then perhaps it's time to add a //separate// 
`constant-expression-cxx20.cpp`. Or, split out the NRVO-specific parts of this 
one into a smaller simpler `constexpr-copy-elision.cpp` which runs in all 
modes, but leave the bulk of this file alone and C++11-only. WDYT?

Ooh, same comment about `constant-expression-cxx(1y->14).cpp`. I do see that as 
evidence in favor of adding a `constant-expression-cxx20.cpp`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99225

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

[PATCH] D99936: [clang][parser] Unify rejecting (non) decl stmt with gnu attributes

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

I think the behavior is correct, but I do think the original diagnostic was 
somewhat better -- an attribute list *can* appear there, if you're lucky. Of 
course, the old diagnostic isn't great either because adding a `;` after the 
attribute isn't going to improve the situation for the user. So I think I'm 
actually fine either way, but maybe @rsmith has a preference.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99936

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

[PATCH] D99645: [flang][driver] Add debug options not requiring semantic checks

[Kiran Chandramohan via Phabricator via cfe-commits]

kiranchandramohan added inline comments.



Comment at: clang/include/clang/Driver/Options.td:4378
   HelpText<"Run the InputOuputTest action. Use for development and testing 
only.">;
+def fdebug_unparse_no_sema : Flag<["-"], "fdebug-unparse-no-sema">, 
Group,
+  HelpText<"Unparse and stop (skips the semantic checks)">;

Does this flag actually mean, parse and construct the parse-tree and unparse 
and stop?

While the current name is OK now, when the rest of the pipeline (codegen, 
linking etc) is there will it be an issue?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99645

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

[PATCH] D99037: [Matrix] Implement C-style explicit type conversions for matrix types

[Saurabh Jha via Phabricator via cfe-commits]

SaurabhJha added inline comments.



Comment at: clang/lib/CodeGen/CGExprScalar.cpp:1324
+
+if (SrcElementTy->isFloatTy() || SrcElementTy->isDoubleTy()) {
+  QualType DstElementType = 
DstType->castAs()->getElementType();

fhahn wrote:
> I think we should support all floating point type here (`isFloatingPointTy`). 
> Basically we want the same rules as for integer types (around line 1418). 
> Perhaps it would be possible to generalize this code in a separate function 
> that takes the LLVM types & the element type/scalar types?
I think we should move this code to `MatrixBuilder` but then generalising 
floating point after that would be problematic since the code is splitted 
across this function and in MatrixBuilder, right?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99037

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

[PATCH] D99262: [analyzer] Fix dead store checker false positive

[Valeriy Savchenko via Phabricator via cfe-commits]

vsavchenko added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/DeadStoresChecker.cpp:420-421
+  // We should also allow defensive initialization of structs.
+  if (const auto *ILE =
+  dyn_cast(E->IgnoreParenCasts())) {
+// We can use exactly the same logic here.

martong wrote:
> What about nested InitListExpr's?
> ```
> std::array a1{ {1, 2, 3} };
> ```
> 
> ```
> VarDecl 0x561b200333a0  col:20 a1 
> 'std::array':'std::array' listinit
> `-InitListExpr 0x561b20036d78  'std::array 3>':'std::array'
>   `-InitListExpr 0x561b20036dc0  'typename 
> _AT_Type::_Type':'int [3]'
> |-IntegerLiteral 0x561b20033408  'int' 1
> |-IntegerLiteral 0x561b20033428  'int' 2
> `-IntegerLiteral 0x561b20033448  'int' 3
> ```
I'm not sure that we'll report anything on that


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99262

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

[PATCH] D99005: [clang] Implement P2266 Simpler implicit move

[Matheus Izvekov via Phabricator via cfe-commits]

mizvekov updated this revision to Diff 335604.
mizvekov added a comment.

- test/SemaCXX/coroutine-rvo: Make MoveOnly trivial, since the temporary is not 
created anymore and there is no special reason to test a non-trivial type here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99005

Files:
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/SemaCoroutine.cpp
  clang/lib/Sema/SemaStmt.cpp
  clang/lib/Sema/SemaType.cpp
  clang/test/CXX/class/class.init/class.copy.elision/p3.cpp
  clang/test/CXX/dcl.dcl/dcl.spec/dcl.type/dcl.spec.auto/p7-cxx14.cpp
  clang/test/CXX/drs/dr3xx.cpp
  clang/test/CXX/expr/expr.prim/expr.prim.lambda/p4-cxx14.cpp
  clang/test/CXX/temp/temp.decls/temp.mem/p5.cpp
  clang/test/SemaCXX/constant-expression-cxx11.cpp
  clang/test/SemaCXX/constant-expression-cxx14.cpp
  clang/test/SemaCXX/coroutine-rvo.cpp
  clang/test/SemaCXX/coroutines.cpp
  clang/test/SemaCXX/deduced-return-type-cxx14.cpp
  clang/test/SemaCXX/return-stack-addr.cpp
  clang/test/SemaCXX/warn-return-std-move.cpp

Index: clang/test/SemaCXX/warn-return-std-move.cpp
===
--- clang/test/SemaCXX/warn-return-std-move.cpp
+++ clang/test/SemaCXX/warn-return-std-move.cpp
@@ -1,8 +1,8 @@
-// RUN: %clang_cc1 -std=c++2b -fsyntax-only -verify=cxx20_2b -fcxx-exceptions -Wreturn-std-move %s
-// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify=cxx20_2b -fcxx-exceptions -Wreturn-std-move %s
-// RUN: %clang_cc1 -std=c++17 -fsyntax-only -verify=cxx11_17 -fcxx-exceptions -Wreturn-std-move %s
-// RUN: %clang_cc1 -std=c++14 -fsyntax-only -verify=cxx11_17 -fcxx-exceptions -Wreturn-std-move %s
-// RUN: %clang_cc1 -std=c++11 -fsyntax-only -verify=cxx11_17 -fcxx-exceptions -Wreturn-std-move %s
+// RUN: %clang_cc1 -std=c++2b -fsyntax-only -verify=cxx20_2b,cxx2b -fcxx-exceptions -Wreturn-std-move %s
+// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify=cxx20_2b   -fcxx-exceptions -Wreturn-std-move %s
+// RUN: %clang_cc1 -std=c++17 -fsyntax-only -verify=cxx11_17   -fcxx-exceptions -Wreturn-std-move %s
+// RUN: %clang_cc1 -std=c++14 -fsyntax-only -verify=cxx11_17   -fcxx-exceptions -Wreturn-std-move %s
+// RUN: %clang_cc1 -std=c++11 -fsyntax-only -verify=cxx11_17   -fcxx-exceptions -Wreturn-std-move %s
 
 // RUN: %clang_cc1 -std=c++17 -fsyntax-only -fcxx-exceptions -Wreturn-std-move -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s -check-prefix=CHECK
 // RUN: %clang_cc1 -std=c++14 -fsyntax-only -fcxx-exceptions -Wreturn-std-move -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s -check-prefix=CHECK
@@ -217,8 +217,8 @@
 }
 
 // But if the return type is a reference type, then moving would be wrong.
-Derived& testRetRef1(Derived&& d) { return d; }
-Base& testRetRef2(Derived&& d) { return d; }
+Derived (Derived &) { return d; } // cxx2b-error {{on-const lvalue reference to type 'Derived' cannot bind to a temporary of type 'Derived'}}
+Base (Derived &) { return d; }// cxx2b-error {{non-const lvalue reference to type 'Base' cannot bind to a temporary of type 'Derived'}}
 #if __cplusplus >= 201402L
 auto&& testRetRef3(Derived&& d) { return d; }
 decltype(auto) testRetRef4(Derived&& d) { return (d); }
Index: clang/test/SemaCXX/return-stack-addr.cpp
===
--- clang/test/SemaCXX/return-stack-addr.cpp
+++ clang/test/SemaCXX/return-stack-addr.cpp
@@ -1,6 +1,6 @@
-// RUN: %clang_cc1 -std=c++2b -fsyntax-only -verify=expected   %s
-// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify=expected   %s
-// RUN: %clang_cc1 -std=c++11 -fsyntax-only -verify=expected,cxx11 %s
+// RUN: %clang_cc1 -std=c++2b -fsyntax-only -verify=expected,cxx2b  %s
+// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify=expected,cxx11_20   %s
+// RUN: %clang_cc1 -std=c++11 -fsyntax-only -verify=expected,cxx11_20,cxx11 %s
 
 int* ret_local() {
   int x = 1;
@@ -29,7 +29,8 @@
 
 int& ret_local_ref() {
   int x = 1;
-  return x;  // expected-warning {{reference to stack memory}}
+  return x; // cxx11_20-warning {{reference to stack memory}}
+  // cxx2b-error@-1 {{non-const lvalue reference to type 'int' cannot bind to a temporary of type 'int'}}
 }
 
 int* ret_local_addrOf() {
@@ -154,8 +155,10 @@
   (void) [&]() -> int& { return b; };
   (void) [=]() mutable -> int& { return a; };
   (void) [=]() mutable -> int& { return b; };
-  (void) [&]() -> int& { int a; return a; }; // expected-warning {{reference to stack}}
-  (void) [=]() -> int& { int a; return a; }; // expected-warning {{reference to stack}}
+  (void) [&]() -> int& { int a; return a; }; // cxx11_20-warning {{reference to stack}}
+  // cxx2b-error@-1 {{non-const lvalue reference to type 'int' cannot bind to a temporary of type 'int'}}
+  (void) [=]() -> int& { int a; return a; }; // cxx11_20-warning {{reference to stack}}
+  // cxx2b-error@-1 {{non-const lvalue reference to type 

[PATCH] D99225: [clang] tests: cleanup, update and add some new ones

[Matheus Izvekov via Phabricator via cfe-commits]

mizvekov updated this revision to Diff 335599.
mizvekov added a comment.

- Finally addresses all of Arthur's review requests.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99225

Files:
  clang/test/CXX/class/class.init/class.copy.elision/p3.cpp
  clang/test/CXX/dcl.dcl/dcl.spec/dcl.type/dcl.spec.auto/p7-1y.cpp
  clang/test/CXX/dcl.dcl/dcl.spec/dcl.type/dcl.spec.auto/p7-cxx14.cpp
  clang/test/CXX/drs/dr3xx.cpp
  clang/test/CXX/expr/expr.prim/expr.prim.lambda/p4-1y.cpp
  clang/test/CXX/expr/expr.prim/expr.prim.lambda/p4-cxx14.cpp
  clang/test/CXX/special/class.copy/p3-cxx11.cpp
  clang/test/CXX/special/class.copy/p33-0x.cpp
  clang/test/CXX/temp/temp.decls/temp.mem/p5.cpp
  clang/test/CodeGen/nrvo-tracking.cpp
  clang/test/SemaCXX/P1155.cpp
  clang/test/SemaCXX/constant-expression-cxx11.cpp
  clang/test/SemaCXX/constant-expression-cxx14.cpp
  clang/test/SemaCXX/constant-expression-cxx1y.cpp
  clang/test/SemaCXX/conversion-function.cpp
  clang/test/SemaCXX/coroutine-rvo.cpp
  clang/test/SemaCXX/coroutines.cpp
  clang/test/SemaCXX/cxx1y-deduced-return-type.cpp
  clang/test/SemaCXX/deduced-return-type-cxx14.cpp
  clang/test/SemaCXX/return-stack-addr.cpp
  clang/test/SemaCXX/warn-return-std-move.cpp

Index: clang/test/SemaCXX/warn-return-std-move.cpp
===
--- clang/test/SemaCXX/warn-return-std-move.cpp
+++ clang/test/SemaCXX/warn-return-std-move.cpp
@@ -1,10 +1,12 @@
-// RUN: %clang_cc1 -fsyntax-only -fcxx-exceptions -Wreturn-std-move -std=c++20 -verify=cxx20 %s
-// RUN: %clang_cc1 -fsyntax-only -fcxx-exceptions -Wreturn-std-move -std=c++17 -verify=expected %s
-// RUN: %clang_cc1 -fsyntax-only -fcxx-exceptions -Wreturn-std-move -std=c++14 -verify=expected %s
-// RUN: %clang_cc1 -fsyntax-only -fcxx-exceptions -Wreturn-std-move -std=c++11 -verify=expected %s
-// RUN: %clang_cc1 -fsyntax-only -fcxx-exceptions -Wreturn-std-move -std=c++17 -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s -check-prefix=CHECK
-// RUN: %clang_cc1 -fsyntax-only -fcxx-exceptions -Wreturn-std-move -std=c++14 -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s -check-prefix=CHECK
-// RUN: %clang_cc1 -fsyntax-only -fcxx-exceptions -Wreturn-std-move -std=c++11 -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s -check-prefix=CHECK
+// RUN: %clang_cc1 -std=c++2b -fsyntax-only -verify=cxx20_2b -fcxx-exceptions -Wreturn-std-move %s
+// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify=cxx20_2b -fcxx-exceptions -Wreturn-std-move %s
+// RUN: %clang_cc1 -std=c++17 -fsyntax-only -verify=cxx11_17 -fcxx-exceptions -Wreturn-std-move %s
+// RUN: %clang_cc1 -std=c++14 -fsyntax-only -verify=cxx11_17 -fcxx-exceptions -Wreturn-std-move %s
+// RUN: %clang_cc1 -std=c++11 -fsyntax-only -verify=cxx11_17 -fcxx-exceptions -Wreturn-std-move %s
+
+// RUN: %clang_cc1 -std=c++17 -fsyntax-only -fcxx-exceptions -Wreturn-std-move -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s -check-prefix=CHECK
+// RUN: %clang_cc1 -std=c++14 -fsyntax-only -fcxx-exceptions -Wreturn-std-move -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s -check-prefix=CHECK
+// RUN: %clang_cc1 -std=c++11 -fsyntax-only -fcxx-exceptions -Wreturn-std-move -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s -check-prefix=CHECK
 
 // definitions for std::move
 namespace std {
@@ -76,8 +78,8 @@
 Base test2() {
 Derived d2;
 return d2;  // e1
-// expected-warning@-1{{will be copied despite being returned by name}}
-// expected-note@-2{{to avoid copying}}
+// cxx11_17-warning@-1{{will be copied despite being returned by name}}
+// cxx11_17-note@-2{{to avoid copying}}
 // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:12-[[@LINE-3]]:14}:"std::move(d2)"
 }
 ConstructFromDerived test3() {
@@ -87,22 +89,22 @@
 ConstructFromBase test4() {
 Derived d4;
 return d4;  // e3
-// expected-warning@-1{{will be copied despite being returned by name}}
-// expected-note@-2{{to avoid copying}}
+// cxx11_17-warning@-1{{will be copied despite being returned by name}}
+// cxx11_17-note@-2{{to avoid copying}}
 // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:12-[[@LINE-3]]:14}:"std::move(d4)"
 }
 ConvertFromDerived test5() {
 Derived d5;
 return d5;  // e4
-// expected-warning@-1{{will be copied despite being returned by name}}
-// expected-note@-2{{to avoid copying}}
+// cxx11_17-warning@-1{{will be copied despite being returned by name}}
+// cxx11_17-note@-2{{to avoid copying}}
 // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:12-[[@LINE-3]]:14}:"std::move(d5)"
 }
 ConvertFromBase test6() {
 Derived d6;
 return d6;  // e5
-// expected-warning@-1{{will be copied despite being returned by name}}
-// expected-note@-2{{to avoid copying}}
+// cxx11_17-warning@-1{{will be copied despite being returned by name}}
+// cxx11_17-note@-2{{to avoid copying}}
 // CHECK: 

[PATCH] D99225: [clang] tests: cleanup, update and add some new ones

[Matheus Izvekov via Phabricator via cfe-commits]

mizvekov added inline comments.



Comment at: clang/test/SemaCXX/coroutine-rvo.cpp:41-45
 struct MoveOnly {
-  MoveOnly() {};
+  MoveOnly() = default;
   MoveOnly(const MoveOnly&) = delete;
-  MoveOnly(MoveOnly&&) noexcept {};
-  ~MoveOnly() {};
+  MoveOnly(MoveOnly &&) = default;
+};

mizvekov wrote:
> Quuxplusone wrote:
> > This change makes `is_trivial_v` true, where before it was false.
> > I don't know whether this matters to the coroutine machinery. Ideally, 
> > @aaronpuchert would weigh in; I don't know, and am just raising the issue 
> > because it struck my attention.
> > 
> > (If it //does// matter, then maybe we even want to add test coverage for 
> > //both// the move-only-trivial case and the move-only-nontrivial case.)
> So these are the changes I picked from D68845.
> 
> You were a reviewer, and you did not raise an objection there :)
> 
> Though that is perfectly fine, that was a couple of years back and our 
> perceptions just change I guess.
> 
> Though once I have P2266 implementation rebased on top of this, I can see if 
> this change still makes sense in our context, but I'd also like for 
> @aaronpuchert to weight on this.
I think I see why this change was made. Both in @aaronpuchert 's patch and in 
the P2266 implementation, we stop creating that (incorrect) temporary when 
passing value between co_return expression and return_value. A non-trivial 
value is a better test for the temporary, but it does not add anything to the 
case with no temporary.

So what I am going to do, is to move just this change to the P2266 
implementation patch.



Comment at: clang/test/SemaCXX/coroutine-rvo.cpp:144
+
+template_return_task param2template(MoveOnly value) {
+  co_return value; // We should deduce U = MoveOnly.

mizvekov wrote:
> Quuxplusone wrote:
> > Nit: `return_template_task` or `generic_task` would look less confusingly 
> > like `template return_task...` at first glance. :)
> Like previous, you did not raise an objection on the original D68845 :)
> 
> But I agree with this change. But I'd like for @aaronpuchert to have a final 
> say on this since these are his tests that I am merely taking, just so they 
> do not sit there unused.
I will go ahead and make this change anyway, since he might be unavailable. We 
can always revert it if he comes back and raises an objection, I guess :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99225

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

[PATCH] D99194: [analyzer] Fix body farm for Obj-C++ properties

[Valeriy Savchenko via Phabricator via cfe-commits]

vsavchenko added inline comments.



Comment at: clang/lib/Analysis/BodyFarm.cpp:757
+  if (PI->getPropertyDecl()) {
+Prop = PI->getPropertyDecl();
+if (Prop->getGetterName() == MD->getSelector())

NoQ wrote:
> At this point `Prop` may contain a completely unrelated property decl. It 
> doesn't necessarily correspond to the `IVar` or have the right name. We keep 
> it after the end of the loop and use it later. This doesn't sound right.
It is actually intentional.  To reuse that bit of code that used to be under 
`if(!IVar)` condition.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99194

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

[PATCH] D97669: [clang][AVR] Add avr-libc/include to clang system include paths

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added inline comments.



Comment at: clang/lib/Driver/ToolChains/AVR.cpp:356
 
+void AVRToolChain::AddClangSystemIncludeArgs(const ArgList ,
+ ArgStringList ) const {

benshi001 wrote:
> Anastasia wrote:
> > function name doesn't adhere to the coding style.
> Not sure your concern ? Other platforms also use the same function name.
> 
> ```
> void RISCVToolChain::AddClangSystemIncludeArgs(const ArgList ,
>ArgStringList ) const {
> void Linux::AddClangSystemIncludeArgs(const ArgList ,
>   ArgStringList ) const {
> void WebAssembly::AddClangSystemIncludeArgs(const ArgList ,
> ArgStringList ) const {
> ```
https://llvm.org/docs/CodingStandards.html#name-types-functions-variables-and-enumerators-properly



> Function names should be verb phrases (as they represent actions), and 
> command-like function should be imperative. The name should be camel case, 
> and start with a lower case letter (e.g. openFile() or isFoo()).


You should adhere to the coding style in the new functionality.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97669

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

[PATCH] D99488: [SYCL][Doc] Add address space handling section to SYCL documentation

[Alexey Bader via Phabricator via cfe-commits]

bader updated this revision to Diff 335596.
bader marked 7 inline comments as done.
bader added a comment.

Add ReST marks to hyperlinks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99488

Files:
  clang/docs/SYCLSupport.rst


Index: clang/docs/SYCLSupport.rst
===
--- clang/docs/SYCLSupport.rst
+++ clang/docs/SYCLSupport.rst
@@ -219,3 +219,93 @@
 
 Additional details of kernel parameter passing may be found in the document
 `SYCL Kernel Parameter Handling and Array Support 
`_.
+
+Address space handling
+^^
+
+The SYCL specification represents pointers to disjoint memory regions using C++
+wrapper classes on an accelerator to enable compilation with a standard C++
+toolchain and a SYCL compiler toolchain. Section 3.8.2 of SYCL 2020
+specification defines
+`memory model 
`_\
 ,
+section 4.7.7 - `address space classes 
`_
+and section 5.9 covers `address space deduction 
`_.
+The SYCL specification allows two modes of address space deduction: "generic as
+default address space" (see section 5.9.3) and "inferred address space" (see
+section 5.9.4). Current implementation supports only "generic as default 
address
+space" mode.
+
+SYCL borrows its memory model from OpenCL however SYCL doesn't perform
+the address space qualifier inference as detailed in
+`OpenCL C v3.0 6.7.8 
`_.
+
+The default address space is "generic-memory", which is a virtual address space
+that overlaps the global, local, and private address spaces. SYCL mode enables
+both explicit and implicit conversion to/from the default address space from/to
+the address space-attributed type. All named address spaces are disjoint and
+sub-sets of default address space.
+
+The SPIR target allocates SYCL namespace scope variables in the global address
+space.
+
+Pointers to default address space should get lowered into a pointer to a 
generic
+address space (or flat to reuse more general terminology). But depending on the
+allocation context, the default address space of a non-pointer type is assigned
+to a specific address space. This is described in
+`common address space deduction rules 
`_
+section.
+
+This is also in line with the behaviour of CUDA (`small example
+`_).
+
+``multi_ptr`` class implementation example:
+
+.. code-block:: C++
+
+   // check that SYCL mode is ON and we can use non-standard decorations
+   #if defined(__SYCL_DEVICE_ONLY__)
+   // GPU/accelerator implementation
+   template  class multi_ptr {
+ // DecoratedType applies corresponding address space attribute to the 
type T
+ // DecoratedType::type == 
"__attribute__((opencl_global)) T"
+ // See sycl/include/CL/sycl/access/access.hpp for more details
+ using pointer_t = typename DecoratedType::type *;
+
+ pointer_t m_Pointer;
+ public:
+ pointer_t get() { return m_Pointer; }
+ T& operator* () { return *reinterpret_cast(m_Pointer); }
+   }
+   #else
+   // CPU/host implementation
+   template  class multi_ptr {
+ T *m_Pointer; // regular undecorated pointer
+ public:
+ T *get() { return m_Pointer; }
+ T& operator* () { return *m_Pointer; }
+   }
+   #endif
+
+Depending on the compiler mode, ``multi_ptr`` will either decorate its internal
+data with the address space attribute or not.
+
+To utilize clang's existing functionality, we reuse the following OpenCL 
address
+space attributes for pointers:
+
+.. list-table::
+   :header-rows: 1
+
+   * - Address space attribute
+ - SYCL address_space enumeration
+   * - ``__attribute__((opencl_global))``
+ - global_space, constant_space
+   * - ``__attribute__((opencl_local))``
+ - local_space
+   * - ``__attribute__((opencl_private))``
+ - private_space
+
+
+.. code-block::
+
+   TODO: add support for `__attribute__((opencl_global_host))` and
+   `__attribute__((opencl_global_device))`.


Index: clang/docs/SYCLSupport.rst
===
--- clang/docs/SYCLSupport.rst
+++ clang/docs/SYCLSupport.rst
@@ -219,3 +219,93 @@
 
 Additional details of kernel parameter passing may be found in the document
 `SYCL Kernel Parameter Handling and Array Support `_.
+
+Address space 

[PATCH] D96853: [clang][AVR] Support variable decorator '__flash'

[Anastasia Stulova via Phabricator via cfe-commits]

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

LGTM!


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

https://reviews.llvm.org/D96853

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

[PATCH] D99969: [OpenCL] Accept .rgba in OpenCL 3.0

[Anastasia Stulova via Phabricator via cfe-commits]

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

LGTM! Thanks!

The code formating check reported an issue though...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99969

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

[PATCH] D99742: [RISCV][Clang] Add RVV Type-Convert intrinsic functions.

[Craig Topper via Phabricator via cfe-commits]

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

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99742

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

[PATCH] D99675: RFC [llvm][clang] Create new intrinsic llvm.arith.fence to control FP optimization at expression level

[Eli Friedman via Phabricator via cfe-commits]

efriedma added a comment.

> The expression “llvm.arith.fence(a * b) + c” means that “a * b” must happen 
> before “+ c” and FMA guarantees that, but to prevent later optimizations from 
> unpacking the FMA the correct transformation needs to be:
>
> llvm.arith.fence(a * b) + c  →  llvm.arith.fence(FMA(a, b, c))

Does this actually block later transforms from unpacking the FMA?  Maybe if the 
FMA isn't marked "fast"...



How is llvm.arith.fence() different from using "freeze" on a floating-point 
value?  The goal isn't really the same, sure, but the effects seem similar at 
first glance.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99675

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

[PATCH] D99488: [SYCL][Doc] Add address space handling section to SYCL documentation

[Anastasia Stulova via Phabricator via cfe-commits]

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

LGTM! Thanks for working on this. The expected sematic seems fairly clear now. 
We might add a few more details while refining the implementation but it should 
not block the development progress at this point.




Comment at: clang/docs/SYCLSupport.rst:255
+to a specific address space. This is described in
+https://www.khronos.org/registry/SYCL/specs/sycl-2020/html/sycl-2020.html#subsec:commonAddressSpace.
+

I think this should be converted into a hyper link?



Comment at: clang/docs/SYCLSupport.rst:258
+This is also in line with the behaviour of CUDA (small example
+https://godbolt.org/z/veqTfo9PK).
+

same here, the list should be converted into a hyper link.



Comment at: clang/docs/SYCLSupport.rst:344
+
+The SPIR target allocates SYCL namespace scope variables in the global address
+space.

bader wrote:
> Anastasia wrote:
> > Interesting, will this deduction always be target specific or can it be 
> > generalized since it is governed by the language semantic already?
> > Interesting, will this deduction always be target specific or can it be 
> > generalized since it is governed by the language semantic already?
> 
> It's target specific deduction. CPU targets doesn't require such deduction.
That's right we have a similar situation in OpenCL. If you expect certain logic 
to be common enough it might make sense to add abstractions at some point.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99488

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

[PATCH] D99645: [flang][driver] Add debug options not requiring semantic checks

[Andrzej Warzynski via Phabricator via cfe-commits]

awarzynski added inline comments.



Comment at: flang/lib/Frontend/FrontendActions.cpp:127
+
+  auto {*ci.parsing().parseTree()};
+

ashermancinelli wrote:
> Is this variable unused?
How did it get here? :) Thank you, updated!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99645

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