[PATCH] D84387: [AST][RecoveryExpr] Suppress spurious "err_typecheck_expect_scalar_operand" diagnostic

[Haojian Wu via Phabricator via cfe-commits]

hokein created this revision.
hokein added a reviewer: sammccall.
Herald added a project: clang.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D84387

Files:
  clang/lib/Sema/SemaCast.cpp
  clang/test/Sema/error-dependence.c


Index: clang/test/Sema/error-dependence.c
===
--- clang/test/Sema/error-dependence.c
+++ clang/test/Sema/error-dependence.c
@@ -1,6 +1,6 @@
 // RUN: %clang_cc1 -fsyntax-only -verify -frecovery-ast -fno-recovery-ast-type 
-fc-dependence %s
 
-int call(int); // expected-note {{'call' declared here}}
+int call(int); // expected-note 2{{'call' declared here}}
 
 void test1(int s) {
   // verify "assigning to 'int' from incompatible type ''" is
@@ -12,8 +12,14 @@
   (*__builtin_classify_type)(1); // expected-error {{builtin functions must be 
directly called}}
 }
 
-void test2(int* ptr, float f) {
+void test2(int *ptr, float f) {
   // verify diagnostic "used type '' where arithmetic or 
pointer
   // type is required" is not emitted.
   ptr > f ? ptr : f; // expected-error {{invalid operands to binary 
expression}}
 }
+
+void test3(float f) {
+  // verify diagnostic "operand of type '' where arithmetic or
+  // pointer type is required" is not emitted.
+  f = (float)call(); // expected-error {{too few arguments to function call}}
+}
Index: clang/lib/Sema/SemaCast.cpp
===
--- clang/lib/Sema/SemaCast.cpp
+++ clang/lib/Sema/SemaCast.cpp
@@ -2690,6 +2690,16 @@
 return;
   }
 
+  if (Self.getLangOpts().CDependence &&
+  (DestType->isDependentType() || SrcExpr.get()->isTypeDependent() ||
+   SrcExpr.get()->isValueDependent())) {
+assert((DestType->containsErrors() || SrcExpr.get()->containsErrors() ||
+SrcExpr.get()->containsErrors()) &&
+   "should only occur in error-recovery path.");
+assert(Kind == CK_Dependent);
+return;
+  }
+
   // Overloads are allowed with C extensions, so we need to support them.
   if (SrcExpr.get()->getType() == Self.Context.OverloadTy) {
 DeclAccessPair DAP;


Index: clang/test/Sema/error-dependence.c
===
--- clang/test/Sema/error-dependence.c
+++ clang/test/Sema/error-dependence.c
@@ -1,6 +1,6 @@
 // RUN: %clang_cc1 -fsyntax-only -verify -frecovery-ast -fno-recovery-ast-type -fc-dependence %s
 
-int call(int); // expected-note {{'call' declared here}}
+int call(int); // expected-note 2{{'call' declared here}}
 
 void test1(int s) {
   // verify "assigning to 'int' from incompatible type ''" is
@@ -12,8 +12,14 @@
   (*__builtin_classify_type)(1); // expected-error {{builtin functions must be directly called}}
 }
 
-void test2(int* ptr, float f) {
+void test2(int *ptr, float f) {
   // verify diagnostic "used type '' where arithmetic or pointer
   // type is required" is not emitted.
   ptr > f ? ptr : f; // expected-error {{invalid operands to binary expression}}
 }
+
+void test3(float f) {
+  // verify diagnostic "operand of type '' where arithmetic or
+  // pointer type is required" is not emitted.
+  f = (float)call(); // expected-error {{too few arguments to function call}}
+}
Index: clang/lib/Sema/SemaCast.cpp
===
--- clang/lib/Sema/SemaCast.cpp
+++ clang/lib/Sema/SemaCast.cpp
@@ -2690,6 +2690,16 @@
 return;
   }
 
+  if (Self.getLangOpts().CDependence &&
+  (DestType->isDependentType() || SrcExpr.get()->isTypeDependent() ||
+   SrcExpr.get()->isValueDependent())) {
+assert((DestType->containsErrors() || SrcExpr.get()->containsErrors() ||
+SrcExpr.get()->containsErrors()) &&
+   "should only occur in error-recovery path.");
+assert(Kind == CK_Dependent);
+return;
+  }
+
   // Overloads are allowed with C extensions, so we need to support them.
   if (SrcExpr.get()->getType() == Self.Context.OverloadTy) {
 DeclAccessPair DAP;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D84387: [AST][RecoveryExpr] Suppress spurious "err_typecheck_expect_scalar_operand" diagnostic

[Haojian Wu via Phabricator via cfe-commits]

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



Comment at: clang/lib/Sema/SemaCast.cpp:2695
+  (DestType->isDependentType() || SrcExpr.get()->isTypeDependent() ||
+   SrcExpr.get()->isValueDependent())) {
+assert((DestType->containsErrors() || SrcExpr.get()->containsErrors() ||

This is similar to the way how C++ codepath handles dependent code, 
https://github.com/llvm/llvm-project/blob/master/clang/lib/Sema/SemaCast.cpp#L2527-L2532.
 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84387



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

[PATCH] D84343: [AST] Keep FP options in trailing storage of CallExpr

[Serge Pavlov via Phabricator via cfe-commits]

sepavloff updated this revision to Diff 280040.
sepavloff added a comment.

Extended test with PCH read


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84343

Files:
  clang/include/clang/AST/Expr.h
  clang/include/clang/AST/ExprCXX.h
  clang/include/clang/AST/Stmt.h
  clang/include/clang/AST/TextNodeDumper.h
  clang/lib/AST/ASTImporter.cpp
  clang/lib/AST/Expr.cpp
  clang/lib/AST/ExprCXX.cpp
  clang/lib/AST/TextNodeDumper.cpp
  clang/lib/Analysis/BodyFarm.cpp
  clang/lib/Frontend/Rewrite/RewriteModernObjC.cpp
  clang/lib/Frontend/Rewrite/RewriteObjC.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/lib/Sema/SemaOpenMP.cpp
  clang/lib/Sema/SemaOverload.cpp
  clang/lib/Sema/TreeTransform.h
  clang/lib/Serialization/ASTReaderStmt.cpp
  clang/lib/Serialization/ASTWriterStmt.cpp
  clang/test/AST/ast-dump-fpfeatures.cpp

Index: clang/test/AST/ast-dump-fpfeatures.cpp
===
--- /dev/null
+++ clang/test/AST/ast-dump-fpfeatures.cpp
@@ -0,0 +1,37 @@
+// Test without serialization:
+// RUN: %clang_cc1 -fsyntax-only -triple x86_64-pc-linux -std=c++11 -ast-dump %s \
+// RUN: | FileCheck --strict-whitespace %s
+
+// Test with serialization:
+// RUN: %clang_cc1 -triple x86_64-pc-linux -emit-pch -o %t %s
+// RUN: %clang_cc1 -x c++ -triple x86_64-pc-linux -include-pch %t -ast-dump-all /dev/null \
+// RUN: | sed -e "s/ //" -e "s/ imported//" \
+// RUN: | FileCheck --strict-whitespace %s
+
+float func_01(float x);
+
+template 
+T func_02(T x) {
+#pragma STDC FP_CONTRACT ON
+  return func_01(x);
+}
+
+float func_03(float x) {
+#pragma STDC FP_CONTRACT OFF
+  return func_02(x);
+}
+
+// CHECK:  FunctionTemplateDecl {{.*}} func_02
+// CHECK:FunctionDecl {{.*}} func_02 'float (float)'
+// CHECK-NEXT: TemplateArgument type 'float'
+// CHECK-NEXT:   BuiltinType {{.*}} 'float'
+// CHECK-NEXT: ParmVarDecl {{.*}} x 'float'
+// CHECK-NEXT: CompoundStmt
+// CHECK-NEXT:   ReturnStmt
+// CHECK-NEXT: CallExpr {{.*}} FPContractMode=1
+
+// CHECK:  FunctionDecl {{.*}} func_03 'float (float)'
+// CHECK-NEXT:   ParmVarDecl {{.*}} x 'float'
+// CHECK-NEXT: CompoundStmt
+// CHECK-NEXT:   ReturnStmt
+// CHECK-NEXT: CallExpr {{.*}} FPContractMode=0
\ No newline at end of file
Index: clang/lib/Serialization/ASTWriterStmt.cpp
===
--- clang/lib/Serialization/ASTWriterStmt.cpp
+++ clang/lib/Serialization/ASTWriterStmt.cpp
@@ -848,12 +848,15 @@
 void ASTStmtWriter::VisitCallExpr(CallExpr *E) {
   VisitExpr(E);
   Record.push_back(E->getNumArgs());
+  Record.push_back(E->hasStoredFPFeatures());
   Record.AddSourceLocation(E->getRParenLoc());
   Record.AddStmt(E->getCallee());
   for (CallExpr::arg_iterator Arg = E->arg_begin(), ArgEnd = E->arg_end();
Arg != ArgEnd; ++Arg)
 Record.AddStmt(*Arg);
   Record.push_back(static_cast(E->getADLCallKind()));
+  if (E->hasStoredFPFeatures())
+Record.push_back(E->getFPFeatures().getAsOpaqueInt());
   Code = serialization::EXPR_CALL;
 }
 
@@ -1550,7 +1553,6 @@
   VisitCallExpr(E);
   Record.push_back(E->getOperator());
   Record.AddSourceRange(E->Range);
-  Record.push_back(E->getFPFeatures().getAsOpaqueInt());
   Code = serialization::EXPR_CXX_OPERATOR_CALL;
 }
 
Index: clang/lib/Serialization/ASTReaderStmt.cpp
===
--- clang/lib/Serialization/ASTReaderStmt.cpp
+++ clang/lib/Serialization/ASTReaderStmt.cpp
@@ -991,12 +991,15 @@
 void ASTStmtReader::VisitCallExpr(CallExpr *E) {
   VisitExpr(E);
   unsigned NumArgs = Record.readInt();
+  bool HasFPFeatures = Record.readInt();
   assert((NumArgs == E->getNumArgs()) && "Wrong NumArgs!");
   E->setRParenLoc(readSourceLocation());
   E->setCallee(Record.readSubExpr());
   for (unsigned I = 0; I != NumArgs; ++I)
 E->setArg(I, Record.readSubExpr());
   E->setADLCallKind(static_cast(Record.readInt()));
+  if (HasFPFeatures)
+E->setStoredFPFeatures(FPOptionsOverride(Record.readInt()));
 }
 
 void ASTStmtReader::VisitCXXMemberCallExpr(CXXMemberCallExpr *E) {
@@ -1662,7 +1665,6 @@
   VisitCallExpr(E);
   E->CXXOperatorCallExprBits.OperatorKind = Record.readInt();
   E->Range = Record.readSourceRange();
-  E->setFPFeatures(FPOptionsOverride(Record.readInt()));
 }
 
 void ASTStmtReader::VisitCXXRewrittenBinaryOperator(
@@ -2977,7 +2979,8 @@
 
 case EXPR_CALL:
   S = CallExpr::CreateEmpty(
-  Context, /*NumArgs=*/Record[ASTStmtReader::NumExprFields], Empty);
+  Context, /*NumArgs=*/Record[ASTStmtReader::NumExprFields],
+  /*HasFPFeatures=*/Record[ASTStmtReader::NumExprFields + 1], Empty);
   break;
 
 case EXPR_RECOVERY:
@@ -3585,12 +3588,14 @@
 
 case EXPR_CXX_OPERATOR_CALL:
   S = CXXOperatorCallExpr::CreateEmpty(
-  Context, /*NumArgs=*/Record[A

[PATCH] D84316: [analyzer][NFC] Split CStringChecker to modeling and reporting

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

steakhal added a comment.

In D84316#2168462 , @NoQ wrote:

> Shared accessors look amazing.
>
> [...] you're splitting up the part which performs boring bookkeeping for the 
> state trait from the part which models `strlen()` and other various functions.


Exactly.

> Such separation also looks amazing because ultimately the first part can be 
> made checker-inspecific (i.e., a reusable half-baked trait that can be 
> instantiated multiple times to track various things regardless of their 
> meaning).

Could you elaborate on the latter part? (instantiated multiple times...)

> I don't think i understand having `unix.cstring.CStringChecker` as one more 
> entry in `Checkers.td`. Do you expect there to be a situation when enabling 
> `CStringModeling` without `CStringChecker` actually makes sense?

You seem to be right.
Enabling only the cstring modeling part does not make much sense without 
enabling //at least// the CStringChecker to model the cstring manipulation 
functions - even if the reporting is disabled of such functions.

> If not, why not keep them agglutinated?

I wanted to have a separate class for bookkeeping while minimalizing the 
necessary changes.
What do you think would be the best way to organize this separation?

Few notes:

- Checkers are implemented in the anonymous namespace, so only the given file 
has access to them.
- I wanted to separate the bookkeeping logic from the reporting/function 
modeling logic in different files.
- I like the fact that after the change the CStringChecker implements only the 
evalCall checker callback.

Let me know if I misunderstood something.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84316



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

[PATCH] D82582: [SVE] Remove calls to VectorType::getNumElements from clang

[Cullen Rhodes via Phabricator via cfe-commits]

c-rhodes added inline comments.



Comment at: clang/lib/CodeGen/CGBuiltin.cpp:5612-5613
 
   llvm::VectorType *VTy = GetNeonType(this, Type, HasLegalHalfType, false,
   AllowBFloatArgsAndRet);
   llvm::Type *Ty = VTy;

Is there a reason you can't do a single cast here?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82582



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

[PATCH] D75044: [AArch64] __builtin_return_address for PAuth.

[Daniel Kiss via Phabricator via cfe-commits]

danielkiss updated this revision to Diff 280048.
danielkiss added a comment.

fix review comments.


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

https://reviews.llvm.org/D75044

Files:
  llvm/include/llvm/CodeGen/ISDOpcodes.h
  llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
  llvm/test/CodeGen/AArch64/aarch64-signedreturnaddress.ll

Index: llvm/test/CodeGen/AArch64/aarch64-signedreturnaddress.ll
===
--- /dev/null
+++ llvm/test/CodeGen/AArch64/aarch64-signedreturnaddress.ll
@@ -0,0 +1,31 @@
+; RUN: llc < %s -mtriple=arm64-eabi -asm-verbose=false -mattr=v8.2a | FileCheck %s
+; RUN: llc < %s -mtriple=arm64-eabi -asm-verbose=false -mattr=v8.3a | FileCheck %s --check-prefix=CHECKV83
+
+; Armv8.3-A Pointer Authetication requires a special intsruction to strip the
+; pointer authentication code from the pointer.
+; The XPACLRI instruction assembles to a hint-space instruction before Armv8.3-A
+; therefore this instruction can be safely used for any pre Armv8.3-A architectures.
+; On Armv8.3-A and onwards XPACI is available so use that instead.
+
+define i8* @era0() nounwind readnone #0 {
+entry:
+; CHECK-LABEL: era0:
+; CHECK:  hint #25
+; CHECK-NEXT: str x30, [sp, #-16]!
+; CHECK-NEXT: hint #7
+; CHECK-NEXT: mov x0, x30
+; CHECK-NEXT: ldr x30, [sp], #16
+; CHECK-NEXT: hint #29
+; CHECK-NEXT: ret
+; CHECKV83:   paciasp
+; CHECKV83-NEXT:  str x30, [sp, #-16]!
+; CHECKV83-NEXT:  xpaci   x30
+; CHECKV83-NEXT:  mov x0, x30
+; CHECKV83-NEXT:  ldr x30, [sp], #16
+; CHECKV83-NEXT:  retaa
+  %0 = tail call i8* @llvm.returnaddress(i32 0)
+  ret i8* %0
+}
+attributes #0 = { "sign-return-address"="all" }
+
+declare i8* @llvm.returnaddress(i32) nounwind readnone
Index: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
===
--- llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
+++ llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
@@ -6267,17 +6267,37 @@
   EVT VT = Op.getValueType();
   SDLoc DL(Op);
   unsigned Depth = cast(Op.getOperand(0))->getZExtValue();
+  SDValue ReturnAddress;
   if (Depth) {
 SDValue FrameAddr = LowerFRAMEADDR(Op, DAG);
 SDValue Offset = DAG.getConstant(8, DL, getPointerTy(DAG.getDataLayout()));
-return DAG.getLoad(VT, DL, DAG.getEntryNode(),
-   DAG.getNode(ISD::ADD, DL, VT, FrameAddr, Offset),
-   MachinePointerInfo());
+ReturnAddress = DAG.getLoad(
+VT, DL, DAG.getEntryNode(),
+DAG.getNode(ISD::ADD, DL, VT, FrameAddr, Offset), MachinePointerInfo());
+  } else {
+// Return LR, which contains the return address. Mark it an implicit
+// live-in.
+unsigned Reg = MF.addLiveIn(AArch64::LR, &AArch64::GPR64RegClass);
+ReturnAddress = DAG.getCopyFromReg(DAG.getEntryNode(), DL, Reg, VT);
+  }
+
+  if (!MF.getFunction().hasFnAttribute("sign-return-address"))
+return ReturnAddress;
+
+  // The XPACLRI instruction assembles to a hint-space instruction before
+  // Armv8.3-A therefore this instruction can be safely used for any pre
+  // Armv8.3-A architectures. On Armv8.3-A and onwards XPACI is available so use
+  // that instead.
+  if (Subtarget->hasV8_3aOps()) {
+SDNode *St = DAG.getMachineNode(AArch64::XPACI, DL, VT, ReturnAddress);
+return SDValue(St, 0);
+  } else {
+// XPACLRI operates on LR therefore we must move the operand accordingly.
+SDValue Chain =
+DAG.getCopyToReg(DAG.getEntryNode(), DL, AArch64::LR, ReturnAddress);
+SDNode *St = DAG.getMachineNode(AArch64::XPACLRI, DL, VT, Chain);
+return SDValue(St, 0);
   }
-
-  // Return LR, which contains the return address. Mark it an implicit live-in.
-  unsigned Reg = MF.addLiveIn(AArch64::LR, &AArch64::GPR64RegClass);
-  return DAG.getCopyFromReg(DAG.getEntryNode(), DL, Reg, VT);
 }
 
 /// LowerShiftRightParts - Lower SRA_PARTS, which returns two
Index: llvm/include/llvm/CodeGen/ISDOpcodes.h
===
--- llvm/include/llvm/CodeGen/ISDOpcodes.h
+++ llvm/include/llvm/CodeGen/ISDOpcodes.h
@@ -86,7 +86,16 @@
   /// the parent's frame or return address, and so on.
   FRAMEADDR,
   RETURNADDR,
+
+  /// ADDROFRETURNADDR - Represents the llvm.addressofreturnaddress intrinsic.
+  /// This node takes no operand, returns a target-specific pointer to the
+  /// place in the stack frame where the return address of the current
+  /// function is stored.
   ADDROFRETURNADDR,
+
+  /// SPONENTRY - Represents the llvm.sponentry intrinsic. Takes no argument
+  /// and returns the stack pointer value at the entry of the current
+  /// function calling this intrinsic.
   SPONENTRY,
 
   /// LOCAL_RECOVER - Represents the llvm.localrecover intrinsic.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo

[PATCH] D84306: [clang-format][NFC] Be more careful about the layout of FormatToken.

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added inline comments.



Comment at: clang/lib/Format/FormatToken.h:177
   /// Indicates that this is the first token of the file.
-  bool IsFirst = false;
+  unsigned IsFirst : 1;
 

educate me, why

```
unsigned IsFirst : 1;
```

here and not

```
bool IsFirst : 1;
```

is that equivalent? (I'm literally not sure myself), I wrote a little test just 
to remind myself how this stuff works.

```
#include 

class Foo
{
public:
Foo()
  : A(true)
  , B(false)
  , C(true)
{
}
bool A : 1;
bool B : 1;
bool C : 1;
};

class Bar
{
public:
Bar()
  : A(true)
  , B(false)
  , C(true)
{
}
unsigned A : 1;
unsigned B : 1;
unsigned C : 1;
};

class Fuz
{
public:
Fuz()
  : A(true)
  , B(false)
  , C(true)
{
}
bool A;
bool B;
bool C;
};

class Baz
{
public:
Baz()
  : A(true)
  , B(false)
  , C(true)
{
}
unsigned A;
unsigned B;
unsigned C;
};

int
main(int argc, char *argv[])
{
std::cerr << "Foo " << sizeof(Foo) << "\n";
std::cerr << "Bar " 

[PATCH] D84306: [clang-format][NFC] Be more careful about the layout of FormatToken.

[MyDeveloperDay via Phabricator via cfe-commits]

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

I'm generally in agreement but I think we should let some others comment




Comment at: clang/lib/Format/ContinuationIndenter.cpp:652
+  (Current.isNot(TT_LineComment) ||
+   Previous.getBlockKind() == BK_BracedInit)) {
 State.Stack.back().Indent = State.Column + Spaces;

I think this is better in that its now easier perhaps to see when the block 
kind gets checked:

I wonder if it would read even better as if we added `is(BraceBlockKind)`   
`isNot(BraceBlockKind)` 

e.g.

`Previous.is(BK_BraceInit)`



Comment at: clang/lib/Format/FormatToken.h:151
+BlockKind(BK_Unknown), Type(TT_Unknown), Decision(FD_Unformatted),
+PackingKind(PPK_Inconclusive) {}
 

I much prefer putting the initialization here, I think it makes it MUCH clearer


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84306



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

[PATCH] D83120: [Analyzer][StreamChecker] Use BugType::SuppressOnSink at resource leak report.

[Balázs Kéri via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG65fd651980a8: [Analyzer][StreamChecker] Use 
BugType::SuppressOnSink at resource leak report. (authored by balazske).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83120

Files:
  clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
  clang/test/Analysis/stream.c


Index: clang/test/Analysis/stream.c
===
--- clang/test/Analysis/stream.c
+++ clang/test/Analysis/stream.c
@@ -261,7 +261,9 @@
   if (!F1)
 return;
   if (Test == 1) {
-return; // expected-warning {{Opened stream never closed. Potential 
resource leak}}
+return; // no warning
   }
   rewind(F1);
-} // no warning
+} // expected-warning {{Opened stream never closed. Potential resource leak}}
+// FIXME: This warning should be placed at the `return` above.
+// See https://reviews.llvm.org/D83120 about details.
Index: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
@@ -204,7 +204,8 @@
   BugType BT_IllegalWhence{this, "Illegal whence argument",
"Stream handling error"};
   BugType BT_StreamEof{this, "Stream already in EOF", "Stream handling error"};
-  BugType BT_ResourceLeak{this, "Resource leak", "Stream handling error"};
+  BugType BT_ResourceLeak{this, "Resource leak", "Stream handling error",
+  /*SuppressOnSink =*/true};
 
 public:
   void checkPreCall(const CallEvent &Call, CheckerContext &C) const;
@@ -965,11 +966,6 @@
 ExplodedNode *
 StreamChecker::reportLeaks(const SmallVector &LeakedSyms,
CheckerContext &C, ExplodedNode *Pred) const {
-  // Do not warn for non-closed stream at program exit.
-  // FIXME: Use BugType::SuppressOnSink instead.
-  if (Pred && Pred->getCFGBlock() && Pred->getCFGBlock()->hasNoReturnElement())
-return Pred;
-
   ExplodedNode *Err = C.generateNonFatalErrorNode(C.getState(), Pred);
   if (!Err)
 return Pred;


Index: clang/test/Analysis/stream.c
===
--- clang/test/Analysis/stream.c
+++ clang/test/Analysis/stream.c
@@ -261,7 +261,9 @@
   if (!F1)
 return;
   if (Test == 1) {
-return; // expected-warning {{Opened stream never closed. Potential resource leak}}
+return; // no warning
   }
   rewind(F1);
-} // no warning
+} // expected-warning {{Opened stream never closed. Potential resource leak}}
+// FIXME: This warning should be placed at the `return` above.
+// See https://reviews.llvm.org/D83120 about details.
Index: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
@@ -204,7 +204,8 @@
   BugType BT_IllegalWhence{this, "Illegal whence argument",
"Stream handling error"};
   BugType BT_StreamEof{this, "Stream already in EOF", "Stream handling error"};
-  BugType BT_ResourceLeak{this, "Resource leak", "Stream handling error"};
+  BugType BT_ResourceLeak{this, "Resource leak", "Stream handling error",
+  /*SuppressOnSink =*/true};
 
 public:
   void checkPreCall(const CallEvent &Call, CheckerContext &C) const;
@@ -965,11 +966,6 @@
 ExplodedNode *
 StreamChecker::reportLeaks(const SmallVector &LeakedSyms,
CheckerContext &C, ExplodedNode *Pred) const {
-  // Do not warn for non-closed stream at program exit.
-  // FIXME: Use BugType::SuppressOnSink instead.
-  if (Pred && Pred->getCFGBlock() && Pred->getCFGBlock()->hasNoReturnElement())
-return Pred;
-
   ExplodedNode *Err = C.generateNonFatalErrorNode(C.getState(), Pred);
   if (!Err)
 return Pred;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] 65fd651 - [Analyzer][StreamChecker] Use BugType::SuppressOnSink at resource leak report.

[Balázs Kéri via cfe-commits]

Author: Balázs Kéri
Date: 2020-07-23T11:53:25+02:00
New Revision: 65fd651980a8ad965363807cc334c513e4c8ffe4

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

LOG: [Analyzer][StreamChecker] Use BugType::SuppressOnSink at resource leak 
report.

Summary:
Use the built-in functionality BugType::SuppressOnSink
instead of a manual solution in StreamChecker.

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

Added: 


Modified: 
clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
clang/test/Analysis/stream.c

Removed: 




diff  --git a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp 
b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
index 1e963249156f..6b176b3c4e2b 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
@@ -204,7 +204,8 @@ class StreamChecker : public Checker &LeakedSyms,
CheckerContext &C, ExplodedNode *Pred) const {
-  // Do not warn for non-closed stream at program exit.
-  // FIXME: Use BugType::SuppressOnSink instead.
-  if (Pred && Pred->getCFGBlock() && Pred->getCFGBlock()->hasNoReturnElement())
-return Pred;
-
   ExplodedNode *Err = C.generateNonFatalErrorNode(C.getState(), Pred);
   if (!Err)
 return Pred;

diff  --git a/clang/test/Analysis/stream.c b/clang/test/Analysis/stream.c
index cd5d28ae8455..c57f3159fcc7 100644
--- a/clang/test/Analysis/stream.c
+++ b/clang/test/Analysis/stream.c
@@ -261,7 +261,9 @@ void check_leak_noreturn_2() {
   if (!F1)
 return;
   if (Test == 1) {
-return; // expected-warning {{Opened stream never closed. Potential 
resource leak}}
+return; // no warning
   }
   rewind(F1);
-} // no warning
+} // expected-warning {{Opened stream never closed. Potential resource leak}}
+// FIXME: This warning should be placed at the `return` above.
+// See https://reviews.llvm.org/D83120 about details.



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

[PATCH] D84345: [AMDGPU] Set the default globals address space to 1

[Alexander Richardson via Phabricator via cfe-commits]

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



Comment at: llvm/lib/IR/AutoUpgrade.cpp:4297
+  // address space of 1.
+  if (T.isAMDGPU() && !DL.contains("-G") && !DL.startswith("G")) {
+return DL.empty() ? std::string("G1") : (DL + "-G1").str();

akhuang wrote:
> arichardson wrote:
> > arsenm wrote:
> > > I would expect datalayout upgrades to work by parsing the old string, and 
> > > checking the field values inside. I guess directly checking the string 
> > > isn't a new problem here
> > I agree that would be less error prone. I wonder if there are cases where 
> > the old string may fail to parse so you have to do the textual upgrade 
> > first. I'm happy to make this change.
> > 
> > @akhuang is there a reason you used string parsing in D67631? Any 
> > objections to changing the code to parse the datalayout and add missing 
> > attributes?
> I don't think so; parsing the datalayout sounds better to me too. 
I just looked into parsing the DataLayout instead. Unfortunately the resulting 
code is more complicated since there are no setters in DataLayout and no way to 
create a normalized representation.
There's also no way to differentiate between no `-G ` passed and `-G0` so  
something like `e-p:64:64-G0` will be converted to `e-p:64:64-G0-G1`



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84345



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

[PATCH] D81728: [InstCombine] Add target-specific inst combining

[David Zarzycki via Phabricator via cfe-commits]

davezarzycki added a comment.

I have a multi-stage, auto-git-bisecting bot that has identifying this commit 
as the source of a regression on Fedora 32 (x86-64). This commit broke my first 
stage test (release, no asserts). Might a quick fix happen or do we need to 
revert this?

  FAIL: Clang :: CodeGen/aarch64-bf16-ldst-intrinsics.c (7188 of 67650)
   TEST 'Clang :: CodeGen/aarch64-bf16-ldst-intrinsics.c' 
FAILED 
  Script:
  --
  : 'RUN: at line 1';   /tmp/_update_lc/r/bin/clang -cc1 -internal-isystem 
/tmp/_update_lc/r/lib/clang/12.0.0/include -nostdsysteminc -triple 
aarch64-arm-none-eabi -target-feature +neon -target-feature +bf16   -O2 
-emit-llvm /home/dave/ro_s/lp/clang/test/CodeGen/aarch64-bf16-ldst-intrinsics.c 
-o - | /tmp/_update_lc/r/bin/FileCheck 
/home/dave/ro_s/lp/clang/test/CodeGen/aarch64-bf16-ldst-intrinsics.c 
--check-prefixes=CHECK,CHECK64
  : 'RUN: at line 3';   /tmp/_update_lc/r/bin/clang -cc1 -internal-isystem 
/tmp/_update_lc/r/lib/clang/12.0.0/include -nostdsysteminc -triple 
armv8.6a-arm-none-eabi -target-feature +neon -target-feature +bf16 -mfloat-abi 
hard   -O2 -emit-llvm 
/home/dave/ro_s/lp/clang/test/CodeGen/aarch64-bf16-ldst-intrinsics.c -o - | 
/tmp/_update_lc/r/bin/FileCheck 
/home/dave/ro_s/lp/clang/test/CodeGen/aarch64-bf16-ldst-intrinsics.c 
--check-prefixes=CHECK,CHECK32
  --
  Exit Code: 1
  
  Command Output (stderr):
  --
  /home/dave/ro_s/lp/clang/test/CodeGen/aarch64-bf16-ldst-intrinsics.c:14:13: 
error: CHECK32: expected string not found in input
  // CHECK32: %1 = load <4 x bfloat>, <4 x bfloat>* %0, align 2
  ^
  :7:52: note: scanning from here
  define arm_aapcs_vfpcc <4 x bfloat> @test_vld1_bf16(bfloat* readonly %ptr) 
local_unnamed_addr #0 {
 ^
  :10:5: note: possible intended match here
   %vld1 = tail call <4 x bfloat> @llvm.arm.neon.vld1.v4bf16.p0i8(i8* %0, i32 2)
  ^
  /home/dave/ro_s/lp/clang/test/CodeGen/aarch64-bf16-ldst-intrinsics.c:23:13: 
error: CHECK32: expected string not found in input
  // CHECK32: %1 = load <8 x bfloat>, <8 x bfloat>* %0, align 2
  ^
  :18:53: note: scanning from here
  define arm_aapcs_vfpcc <8 x bfloat> @test_vld1q_bf16(bfloat* readonly %ptr) 
local_unnamed_addr #2 {
  ^
  :21:5: note: possible intended match here
   %vld1 = tail call <8 x bfloat> @llvm.arm.neon.vld1.v8bf16.p0i8(i8* %0, i32 2)
  ^
  
  Input file: 
  Check file: 
/home/dave/ro_s/lp/clang/test/CodeGen/aarch64-bf16-ldst-intrinsics.c
  
  -dump-input=help explains the following input dump.
  
  Input was:
  <<
  1: ; ModuleID = 
'/home/dave/ro_s/lp/clang/test/CodeGen/aarch64-bf16-ldst-intrinsics.c'
  2: source_filename = 
"/home/dave/ro_s/lp/clang/test/CodeGen/aarch64-bf16-ldst-intrinsics.c"
  3: target datalayout = 
"e-m:e-p:32:32-Fi8-i64:64-v128:64:128-a:0:32-n32-S64"
  4: target triple = "armv8.6a-arm-none-eabi"
  5:
  6: ; Function Attrs: nounwind readonly
  7: define arm_aapcs_vfpcc <4 x bfloat> @test_vld1_bf16(bfloat* 
readonly %ptr) local_unnamed_addr #0 {
  check:14'0
X~~ error: no match found
  8: entry:
  check:14'0 ~~
  9:  %0 = bitcast bfloat* %ptr to i8*
  check:14'0 ~
 10:  %vld1 = tail call <4 x bfloat> 
@llvm.arm.neon.vld1.v4bf16.p0i8(i8* %0, i32 2)
  check:14'0 
~~
  check:14'1 ?  
possible intended match
 11:  ret <4 x bfloat> %vld1
  check:14'0 ~~~
 12: }
  check:14'0 ~
 13:
  check:14'0 ~
 14: ; Function Attrs: argmemonly nounwind readonly
  check:14'0 ~~
 15: declare <4 x bfloat> @llvm.arm.neon.vld1.v4bf16.p0i8(i8*, i32) 
#1
  check:14'0 
~
 16:
  check:14'0 ~
 17: ; Function Attrs: nounwind readonly
  check:14'0 ~~~
 18: define arm_aapcs_vfpcc <8 x bfloat> @test_vld1q_bf16(bfloat* 
readonly %ptr) local_unnamed_addr #2 {
  check:14'0 
  check:23'0 
X~~ error: no match found
 19: entry:
  check:23'0 ~~
 20:  %0 = bitcast bfloat* %ptr to i8*
  check:23'0 ~
 21:  %vld1 = tail call <8 x bfloat> 
@llvm.arm.neon.vld1.v8bf16.p0i8(i8* %0, i32 

[PATCH] D81678: Introduce noundef attribute at call sites for stricter poison analysis

[Gui Andrade via Phabricator via cfe-commits]

guiand updated this revision to Diff 279949.
guiand added a comment.

Adds additional constraints on `noundef`: Not a `nullptr_t`, not a member 
pointer, and not coerced to a type of larger size. Disabled emitting in return 
position for non-C++ languages (or inside extern "C").

@rsmith, I just didn't address your comment about being conservative with 
black-holeing C++ functions returning undef. At least for msan purposes this 
shouldn't be a *too* much of a problem, but of course this attribute is for 
general use by LLVM passes. I'm just wondering what kind of tradeoff we want to 
make here, I suppose.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81678

Files:
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGen/indirect-noundef.cpp

Index: clang/test/CodeGen/indirect-noundef.cpp
===
--- /dev/null
+++ clang/test/CodeGen/indirect-noundef.cpp
@@ -0,0 +1,30 @@
+// RUN: %clang_cc1 -x c++ -triple x86_64-unknown-unknown -O0 -emit-llvm -o - %s | FileCheck %s
+
+union u1 { int val; };
+
+// CHECK: @indirect_callee_int_ptr = global i32 (i32)*
+int (*indirect_callee_int_ptr)(int);
+// CHECK: @indirect_callee_union_ptr = global i32 (i32)*
+union u1 (*indirect_callee_union_ptr)(union u1);
+
+// CHECK-LABEL: define noundef i32 @{{.*}}indirect_callee_int{{.*}}(i32 noundef %
+int indirect_callee_int(int a) { return a; }
+// CHECK-LABEL: define i32 @{{.*}}indirect_callee_union{{.*}}(i32 %
+union u1 indirect_callee_union(union u1 a) { return a; }
+
+int main() {
+// CHECK: call noundef i32 @{{.*}}indirect_callee_int{{.*}}(i32 noundef 0)
+indirect_callee_int(0);
+// CHECK: call i32 @{{.*}}indirect_callee_union{{.*}}(i32 %
+indirect_callee_union((union u1){0});
+
+indirect_callee_int_ptr = indirect_callee_int;
+indirect_callee_union_ptr = indirect_callee_union;
+
+// CHECK: call noundef i32 %{{.*}}(i32 noundef 0)
+indirect_callee_int_ptr(0);
+// CHECK: call i32 %{{.*}}(i32 %
+indirect_callee_union_ptr((union u1){});
+
+return 0;
+}
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -924,6 +924,7 @@
 
   Opts.DisableFree = Args.hasArg(OPT_disable_free);
   Opts.DiscardValueNames = Args.hasArg(OPT_discard_value_names);
+  Opts.DisableNoundefArgs = Args.hasArg(OPT_disable_noundef_args);
   Opts.DisableTailCalls = Args.hasArg(OPT_mdisable_tail_calls);
   Opts.NoEscapingBlockTailCalls =
   Args.hasArg(OPT_fno_escaping_block_tail_calls);
Index: clang/lib/CodeGen/CGCall.cpp
===
--- clang/lib/CodeGen/CGCall.cpp
+++ clang/lib/CodeGen/CGCall.cpp
@@ -1876,6 +1876,54 @@
   llvm::for_each(NBA->builtinNames(), AddNoBuiltinAttr);
 }
 
+static bool DetermineNoUndef(QualType QTy, CodeGenTypes &Types,
+ const llvm::DataLayout &DL, const ABIArgInfo &AI,
+ bool CheckCoerce = true) {
+  llvm::Type *Ty = Types.ConvertTypeForMem(QTy);
+  if (AI.getKind() == ABIArgInfo::Indirect)
+return true;
+  if (AI.getKind() == ABIArgInfo::Extend)
+return true;
+  if (!DL.typeSizeEqualsStoreSize(Ty))
+// TODO: This will result in a modest amount of values not marked noundef
+// when they could be. We care about values that *invisibly* contain undef
+// bits from the perspective of LLVM IR. 
+return false;
+  if (CheckCoerce && AI.canHaveCoerceToType()) {
+llvm::Type *CoerceTy = AI.getCoerceToType();
+if (DL.getTypeSizeInBits(CoerceTy) > DL.getTypeSizeInBits(Ty))
+  // If we're coercing to a type with a greater size than the canonical one,
+  // we're introducing new undef bits.
+  // Coercing to a type of smaller or equal size is ok, as we know that
+  // there's no internal padding (typeSizeEqualsStoreSize).
+  return false;
+  }
+  if (QTy->isExtIntType())
+return true;
+  if (QTy->isReferenceType())
+return true;
+  if (QTy->isNullPtrType())
+return false;
+  if (QTy->isMemberPointerType())
+// TODO: Some member pointers are `noundef`, but it depends on the ABI. For
+// now, never mark them.
+return false;
+  if (QTy->isScalarType()) {
+if (const ComplexType *Complex = dyn_cast(QTy))
+  return DetermineNoUndef(Complex->getElementType(), Types, DL, AI, false);
+return true;
+  }
+  if (const VectorType *Vector = dyn_cast(QTy))
+return DetermineNoUndef(Vector->getElementType(), Types, DL, AI, false);
+  if (const MatrixType *Matrix = dyn_cast(QTy))
+return DetermineNoUndef(Matrix->getElementType(), Types, DL, AI, false);
+  if (const ArrayType *Array = dyn_cast(

[PATCH] D83912: [llvm-readobj] Update tests because of changes at llvm-readobj behavior

[Elvina Yakubova via Phabricator via cfe-commits]

Elvina added a comment.

In D83912#2161524 , @jhenderson wrote:

> Hi @Elvina,
>
> Just to let you know that I had to fix up three clang tests that were using 
> the old behaviour too, prior to committing. I also noticed that you've got 
> the stack the wrong way around - you needed to fix the tests before changing 
> the behaviour in this case to avoid breaking the codebase, even if it was 
> only briefly.
>
> Anyway, these are now committed. I'll update the bug.


Hi James,
Thank you for your help and suggestions! I really appreciate this. I'll be more 
attentive next time :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83912



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

[PATCH] D81678: Introduce noundef attribute at call sites for stricter poison analysis

[Gui Andrade via Phabricator via cfe-commits]

guiand added a comment.

Just saw your comment about tests as well. The idea was to have all tests 
ported over as part of a separate commit (I linked it in the main patch 
description) and then only to push either commit once both are ready to land.

To make it easier to be sure this specific feature is working correctly, I can 
port over some of those tests to this patch for review purposes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81678



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

[PATCH] D81728: [InstCombine] Add target-specific inst combining

[Sebastian Neubauer via Phabricator via cfe-commits]

Flakebi added a comment.

Thanks for the notification @davezarzycki, an auto-bisecting bot is cool!

This failure should be fixed in b99898c1e9c5d8bade1d898e84604d3241b0087c 
.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81728



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

[PATCH] D82948: [Driver][ARM] Disable unsupported features when nofp arch extension is used

[Victor Campos via Phabricator via cfe-commits]

vhscampos updated this revision to Diff 280072.
vhscampos added a comment.

1. Add comment explaining the MVE-Integer detail.
2. Add another test to check the disabled features.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82948

Files:
  clang/lib/Driver/ToolChains/Arch/ARM.cpp
  clang/test/CodeGen/arm-bf16-softfloat.c
  clang/test/Driver/arm-nofp-disabled-features.c
  llvm/include/llvm/Support/ARMTargetParser.h
  llvm/lib/Support/ARMTargetParser.cpp
  llvm/unittests/Support/TargetParserTest.cpp

Index: llvm/unittests/Support/TargetParserTest.cpp
===
--- llvm/unittests/Support/TargetParserTest.cpp
+++ llvm/unittests/Support/TargetParserTest.cpp
@@ -668,9 +668,10 @@
 testArchExtDependency(const char *ArchExt,
   const std::initializer_list &Expected) {
   std::vector Features;
+  unsigned FPUID;
 
   if (!ARM::appendArchExtFeatures("", ARM::ArchKind::ARMV8_1MMainline, ArchExt,
-  Features))
+  Features, FPUID))
 return false;
 
   return llvm::all_of(Expected, [&](StringRef Ext) {
Index: llvm/lib/Support/ARMTargetParser.cpp
===
--- llvm/lib/Support/ARMTargetParser.cpp
+++ llvm/lib/Support/ARMTargetParser.cpp
@@ -490,9 +490,10 @@
   return ARM::FK_INVALID;
 }
 
-bool ARM::appendArchExtFeatures(
-  StringRef CPU, ARM::ArchKind AK, StringRef ArchExt,
-  std::vector &Features) {
+bool ARM::appendArchExtFeatures(StringRef CPU, ARM::ArchKind AK,
+StringRef ArchExt,
+std::vector &Features,
+unsigned &ArgFPUID) {
 
   size_t StartingNumFeatures = Features.size();
   const bool Negated = stripNegationPrefix(ArchExt);
@@ -527,6 +528,7 @@
 } else {
   FPUKind = getDefaultFPU(CPU, AK);
 }
+ArgFPUID = FPUKind;
 return ARM::getFPUFeatures(FPUKind, Features);
   }
   return StartingNumFeatures != Features.size();
Index: llvm/include/llvm/Support/ARMTargetParser.h
===
--- llvm/include/llvm/Support/ARMTargetParser.h
+++ llvm/include/llvm/Support/ARMTargetParser.h
@@ -250,7 +250,8 @@
 StringRef getArchExtName(uint64_t ArchExtKind);
 StringRef getArchExtFeature(StringRef ArchExt);
 bool appendArchExtFeatures(StringRef CPU, ARM::ArchKind AK, StringRef ArchExt,
-   std::vector &Features);
+   std::vector &Features,
+   unsigned &ArgFPUKind);
 StringRef getHWDivName(uint64_t HWDivKind);
 
 // Information by Name
Index: clang/test/Driver/arm-nofp-disabled-features.c
===
--- /dev/null
+++ clang/test/Driver/arm-nofp-disabled-features.c
@@ -0,0 +1,18 @@
+// RUN: %clang -target arm-arm-none-eabi -mfloat-abi=soft %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-MFLOAT-ABI-SOFT
+// CHECK-MFLOAT-ABI-SOFT: "-target-feature" "-dotprod"
+// CHECK-MFLOAT-ABI-SOFT: "-target-feature" "-fp16fml"
+// CHECK-MFLOAT-ABI-SOFT: "-target-feature" "-bf16"
+// CHECK-MFLOAT-ABI-SOFT: "-target-feature" "-mve"
+// CHECK-MFLOAT-ABI-SOFT: "-target-feature" "-mve.fp"
+// CHECK-MFLOAT-ABI-SOFT: "-target-feature" "-fpregs"
+
+// RUN: %clang -target arm-arm-none-eabi -mfpu=none %s -### 2>&1 | FileCheck %s
+// RUN: %clang -target arm-arm-none-eabi -march=armv8-a+nofp %s -### 2>&1 | FileCheck %s
+// RUN: %clang -target arm-arm-none-eabi -mcpu=cortex-a35+nofp %s -### 2>&1 | FileCheck %s
+// RUN: %clang -target arm-arm-none-eabi -march=armv8-a+nofp+nomve %s -### 2>&1 | FileCheck %s --check-prefixes=CHECK,CHECK-NOMVE
+// RUN: %clang -target arm-arm-none-eabi -mcpu=cortex-a35+nofp+nomve %s -### 2>&1 | FileCheck %s --check-prefixes=CHECK,CHECK-NOMVE
+// CHECK: "-target-feature" "-dotprod"
+// CHECK: "-target-feature" "-fp16fml"
+// CHECK: "-target-feature" "-bf16"
+// CHECK: "-target-feature" "-mve.fp"
+// CHECK-NOMVE: "-target-feature" "-fpregs"
Index: clang/test/CodeGen/arm-bf16-softfloat.c
===
--- clang/test/CodeGen/arm-bf16-softfloat.c
+++ clang/test/CodeGen/arm-bf16-softfloat.c
@@ -1,4 +1,9 @@
-// RUN: not %clang -o %t.out -target arm-arm-eabi -march=armv8-a+bf16 -mfloat-abi=soft -c %s 2>&1 | FileCheck %s
+// RUN: not %clang -target arm-arm-none-eabi -march=armv8-a+bf16 -mfloat-abi=soft -c %s -o %t 2>&1 | FileCheck %s
+// RUN: not %clang -target arm-arm-none-eabi -march=armv8-a+bf16 -mfpu=none -c %s -o %t 2>&1 | FileCheck %s
+// RUN: not %clang -target arm-arm-none-eabi -march=armv8-a+bf16+nofp -c %s -o %t 2>&1 | FileCheck %s
+// RUN: not %clang -target arm-arm-none-eabi -march=armv8-a+bf16+fp+nofp -c %s -o %t 2>&1 | FileCheck %s
+// RUN: %clang -target arm-arm-none-eabi -march=armv8-a+bf16+

[PATCH] D82467: [PowerPC][Power10] Implement Truncate and Store VSX Vector Builtins

[Lei Huang via Phabricator via cfe-commits]

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

LGTM,
Please add tests for BE in llvm/test/CodeGen/PowerPC/builtins-ppc-p10vsx.ll 
prior to committing.




Comment at: llvm/test/CodeGen/PowerPC/builtins-ppc-p10vsx.ll:2
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc -verify-machineinstrs -mtriple=powerpc64le-unknown-linux-gnu \
+; RUN:   -mcpu=pwr10 -ppc-asm-full-reg-names -ppc-vsr-nums-as-vr < %s | \

stefanp wrote:
> NeHuang wrote:
> > Do we also need to run for big endian?
> I don't think it would hurt to add a BE run for this as well. the only thing 
> to watch out for is the fact that this is not actually a new file. This test 
> file already exists on master so I assume that this will be merged in (and 
> not added as a new file). 
I believe this is a new file 🙂 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82467



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

[PATCH] D80961: Ignore template instantiations if not in AsIs mode

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

I reviewed the changes in the patch and they seem reasonable, but this patch is 
hard to have high confidence in because you need to audit all the places where 
the default behavior silently changed and no changes were made in the patch. 
I'm assuming that the code changes were made to cause existing tests to pass, 
but did you verify that changes weren't needed to checks that show no 
behavioral test changes to ensure that it wasn't because the tests have poor 
template test coverage?




Comment at: 
clang-tools-extra/clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp:62
   StatementMatcher LoopVarConversionMatcher =
   traverse(ast_type_traits::TK_AsIs,
implicitCastExpr(hasImplicitDestinationType(isInteger()),

Do we still need this `TK_AsIs` with the below changes?



Comment at: clang/lib/ASTMatchers/ASTMatchFinder.cpp:564
 
+  bool TraversingImplicitTemplateInstantiation = false;
+

This should not be part of the public interface.



Comment at: clang/lib/ASTMatchers/ASTMatchFinder.cpp:570
+
+  struct ImplicitTemplateInstantiationScope {
+ImplicitTemplateInstantiationScope(MatchASTVisitor *V, bool B)

Nor should this, right?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80961



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

[PATCH] D82948: [Driver][ARM] Disable unsupported features when nofp arch extension is used

[David Spickett via Phabricator via cfe-commits]

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

LGTM, I'll let @chill give the final word.




Comment at: clang/lib/Driver/ToolChains/Arch/ARM.cpp:476
+// -mfpu=none, -march=armvX+nofp or -mcpu=X+nofp is *very* similar to
+// -mfloat-abi=soft, only that it should not disable MVE-I.
 Features.insert(Features.end(),

DavidSpickett wrote:
> chill wrote:
> > DavidSpickett wrote:
> > > Why not disable MVE-I? I assume because it's integer only but then why 
> > > does -mfloat-abi=soft disable it?
> > > 
> > > If possible add a regression test for this. In general a test like the 
> > > bf16 test below, but for all the listed extensions would help. Perhaps it 
> > > makes more sense to add a driver test that looks for the "-" bits in 
> > > the -### output instead of doing each extension on its own.
> > > Why not disable MVE-I?
> > 
> > After MVE, "FPU" registers are a separate entity from the FPU.
> > 
> > `-mfpu=none`/`+nofp` disable the FPU. MVE-I does not require an FPU.
> > `-mfloat-abi=soft` disables both the FPU instructions and the FPU registers.
> > MVE-I requires "FPU" registers.
> > 
> > It's possible to define different semantics, but this is the one we agreed 
> > with GCC.
> Got it. @vhscampos  can you add that to the comment?
Thanks, lgtm.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82948



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

[PATCH] D84197: [PowerPC][Power10] 128-bit Vector String Isolate instruction definitions and MC Tests

[Lei Huang via Phabricator via cfe-commits]

lei added a comment.

Why are you removing MC tests for  here?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84197



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

[PATCH] D82502: [PowerPC][Power10] Implement Load VSX Vector and Sign Extend and Zero Extend

[Lei Huang via Phabricator via cfe-commits]

lei added inline comments.



Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:14166
+
+  SDValue LoadOps[] = {LD->getChain(), LD->getBasePtr(), 
DAG.getIntPtrConstant(MemoryType.getScalarSizeInBits(), dl)};
+

nit: indentation.



Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:14169
+ return DAG.getMemIntrinsicNode(PPCISD::LXVRZX, dl,
+ DAG.getVTList(MVT::v1i128, MVT::Other),
+ LoadOps, MemoryType, LD->getMemOperand());

nit: indentation


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82502



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

[PATCH] D84382: [PowerPC][Power10] Cleanup p10vector clang test

[Lei Huang via Phabricator via cfe-commits]

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

LGTM
thx!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84382



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

[PATCH] D83188: [clang-tidy] bugprone-bool-pointer-implicit-conversion doesn't handle members

[Alex Cameron via Phabricator via cfe-commits]

tetsuo-cpp updated this revision to Diff 280082.
tetsuo-cpp added a comment.

Cleanup check conditional.


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

https://reviews.llvm.org/D83188

Files:
  clang-tools-extra/clang-tidy/bugprone/BoolPointerImplicitConversionCheck.cpp
  
clang-tools-extra/test/clang-tidy/checkers/bugprone-bool-pointer-implicit-conversion.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-bool-pointer-implicit-conversion.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/bugprone-bool-pointer-implicit-conversion.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-bool-pointer-implicit-conversion.cpp
@@ -74,9 +74,31 @@
 bool *b;
   } d = { SomeFunction() };
 
-  if (d.b)
+  if (d.b) {
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: dubious check of 'bool *' against 'nullptr'
+// CHECK-FIXES: if (*d.b) {
+  }
+
+  if (d.b) {
 (void)*d.b; // no-warning
+  }
 
-#define CHECK(b) if (b) {}
+#define CHECK(b) \
+  if (b) {   \
+  }
   CHECK(c)
 }
+
+struct H {
+  bool *b;
+  void foo() const {
+if (b) {
+  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: dubious check of 'bool *' against 'nullptr'
+  // CHECK-FIXES: if (*b) {
+}
+
+if (b) {
+  (void)*b; // no-warning
+}
+  }
+};
Index: clang-tools-extra/clang-tidy/bugprone/BoolPointerImplicitConversionCheck.cpp
===
--- clang-tools-extra/clang-tidy/bugprone/BoolPointerImplicitConversionCheck.cpp
+++ clang-tools-extra/clang-tidy/bugprone/BoolPointerImplicitConversionCheck.cpp
@@ -20,53 +20,68 @@
   Finder->addMatcher(
   traverse(
   ast_type_traits::TK_AsIs,
-  ifStmt(hasCondition(findAll(implicitCastExpr(
- unless(hasParent(unaryOperator(hasOperatorName("!",
- hasSourceExpression(expr(
- hasType(pointerType(pointee(booleanType(,
- ignoringParenImpCasts(declRefExpr().bind("expr",
- hasCastKind(CK_PointerToBoolean,
- unless(isInTemplateInstantiation()))
+  ifStmt(
+  hasCondition(findAll(implicitCastExpr(
+  unless(hasParent(unaryOperator(hasOperatorName("!",
+  hasSourceExpression(expr(
+  hasType(pointerType(pointee(booleanType(,
+  ignoringParenImpCasts(anyOf(declRefExpr().bind("expr"),
+  memberExpr().bind("expr"),
+  hasCastKind(CK_PointerToBoolean,
+  unless(isInTemplateInstantiation()))
   .bind("if")),
   this);
 }
 
-void BoolPointerImplicitConversionCheck::check(
-const MatchFinder::MatchResult &Result) {
-  auto *If = Result.Nodes.getNodeAs("if");
-  auto *Var = Result.Nodes.getNodeAs("expr");
-
+static void checkImpl(const MatchFinder::MatchResult &Result, const Expr *Ref,
+  const IfStmt *If,
+  const ast_matchers::internal::Matcher &RefMatcher,
+  ClangTidyCheck &Check) {
   // Ignore macros.
-  if (Var->getBeginLoc().isMacroID())
+  if (Ref->getBeginLoc().isMacroID())
 return;
 
-  // Only allow variable accesses for now, no function calls or member exprs.
+  // Only allow variable accesses and member exprs for now, no function calls.
   // Check that we don't dereference the variable anywhere within the if. This
   // avoids false positives for checks of the pointer for nullptr before it is
   // dereferenced. If there is a dereferencing operator on this variable don't
   // emit a diagnostic. Also ignore array subscripts.
-  const Decl *D = Var->getDecl();
-  auto DeclRef = ignoringParenImpCasts(declRefExpr(to(equalsNode(D;
-  if (!match(findAll(
- unaryOperator(hasOperatorName("*"), hasUnaryOperand(DeclRef))),
+  if (!match(findAll(unaryOperator(hasOperatorName("*"),
+   hasUnaryOperand(RefMatcher))),
  *If, *Result.Context)
.empty() ||
-  !match(findAll(arraySubscriptExpr(hasBase(DeclRef))), *If,
+  !match(findAll(arraySubscriptExpr(hasBase(RefMatcher))), *If,
  *Result.Context)
.empty() ||
   // FIXME: We should still warn if the paremater is implicitly converted to
   // bool.
-  !match(findAll(callExpr(hasAnyArgument(ignoringParenImpCasts(DeclRef,
- *If, *Result.Context)
+  !match(
+   findAll(callExpr(hasAnyArgument(ignoringParenImpCasts(RefMatcher,
+   *If, *Result.Context)
.empty() ||
-  !match(findAll(cxxDeleteExpr(has(ignoringParenImpCasts(expr(DeclRef),
- *If, *Result.Context)
+  !match(
+   findAll(cxxDeleteExpr(has(ignoringParenImpCasts(expr(RefMatcher),
+   *If, *Result.Contex

[PATCH] D83188: [clang-tidy] bugprone-bool-pointer-implicit-conversion doesn't handle members

[Alex Cameron via Phabricator via cfe-commits]

tetsuo-cpp marked an inline comment as done.
tetsuo-cpp added a comment.

@aaron.ballman 
Thanks! I took your suggestion and I think this looks cleaner.


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

https://reviews.llvm.org/D83188



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

[clang] 7bf9e22 - Update make_cxx_dr_status and cxx_dr_status.html for the new release.

[Erich Keane via cfe-commits]

Author: Erich Keane
Date: 2020-07-23T05:50:10-07:00
New Revision: 7bf9e2204960f70e89df3dd0dc768457ddc40498

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

LOG: Update make_cxx_dr_status and cxx_dr_status.html for the new release.

Our latest release is now 11, so update the make_cxx_dr_status and
regenerate the cxx_dr_status.html document.

Added: 


Modified: 
clang/www/cxx_dr_status.html
clang/www/make_cxx_dr_status

Removed: 




diff  --git a/clang/www/cxx_dr_status.html b/clang/www/cxx_dr_status.html
index 65f196ad59d0..c7369525c36f 100755
--- a/clang/www/cxx_dr_status.html
+++ b/clang/www/cxx_dr_status.html
@@ -1504,7 +1504,7 @@ C++ defect report implementation 
status
 https://wg21.link/cwg244";>244
 CD1
 Destructor lookup
-Clang 11
+Clang 11
   
   
 https://wg21.link/cwg245";>245
@@ -2789,7 +2789,7 @@ C++ defect report implementation 
status
 https://wg21.link/cwg458";>458
 C++11
 Hiding of member template parameters by other members
-Clang 11
+Clang 11
   
   
 https://wg21.link/cwg459";>459
@@ -3967,7 +3967,7 @@ C++ defect report implementation 
status
 https://wg21.link/cwg654";>654
 CD1
 Conversions to and from nullptr_t
-Superseded by 1423
+Superseded by 1423
   
   
 https://wg21.link/cwg655";>655
@@ -8353,7 +8353,7 @@ C++ defect report implementation 
status
 https://wg21.link/cwg1423";>1423
 CD3
 Convertibility of nullptr to bool
-Clang 11
+Clang 11
   
   
 https://wg21.link/cwg1424";>1424
@@ -8899,7 +8899,7 @@ C++ defect report implementation 
status
 https://wg21.link/cwg1514";>1514
 C++14
 Ambiguity between enumeration definition and zero-length bit-field
-Clang 11
+Clang 11
   
   
 https://wg21.link/cwg1515";>1515
@@ -10333,7 +10333,7 @@ C++ defect report implementation 
status
 https://wg21.link/cwg1753";>1753
 CD4
 decltype-specifier in nested-name-specifier of 
destructor
-Clang 11
+Clang 11
   
   
 https://wg21.link/cwg1754";>1754
@@ -11611,7 +11611,7 @@ C++ defect report implementation 
status
 https://wg21.link/cwg1966";>1966
 CD4
 Colon following enumeration elaborated-type-specifier
-Clang 11
+Clang 11
   
   
 https://wg21.link/cwg1967";>1967
@@ -11971,7 +11971,7 @@ C++ defect report implementation 
status
 https://wg21.link/cwg2026";>2026
 CD4
 Zero-initialization and constexpr
-Clang 11
+Clang 11
   
   
 https://wg21.link/cwg2027";>2027
@@ -12307,7 +12307,7 @@ C++ defect report implementation 
status
 https://wg21.link/cwg2082";>2082
 CD4
 Referring to parameters in unevaluated operands of default 
arguments
-Clang 11
+Clang 11
   
   
 https://wg21.link/cwg2083";>2083
@@ -12757,7 +12757,7 @@ C++ defect report implementation 
status
 https://wg21.link/cwg2157";>2157
 CD4
 Further disambiguation of enumeration 
elaborated-type-specifier
-Clang 11
+Clang 11
   
   
 https://wg21.link/cwg2158";>2158
@@ -13213,7 +13213,7 @@ C++ defect report implementation 
status
 https://wg21.link/cwg2233";>2233
 DRWP
 Function parameter packs following default arguments
-Clang 11
+Clang 11
   
   
 https://wg21.link/cwg2234";>2234
@@ -13891,7 +13891,7 @@ C++ defect report implementation 
status
 https://wg21.link/cwg2346";>2346
 DRWP
 Local variables in default arguments
-Clang 11
+Clang 11
   
   
 https://wg21.link/cwg2347";>2347
@@ -14499,6 +14499,24 @@ C++ defect report implementation 
status
 Unintended description of abbreviated function templates
 Unknown
   
+  
+https://wg21.link/cwg2448";>2448
+open
+Cv-qualification of arithmetic types and deprecation of volatile
+Not resolved
+  
+  
+https://wg21.link/cwg2449";>2449
+open
+Thunks as an implementation technique for pointers to virtual 
functions
+Not resolved
+  
+  
+https://wg21.link/cwg2450";>2450
+open
+braced-init-list as a template-argument
+Not resolved
+  
 
 
 

diff  --git a/clang/www/make_cxx_dr_status b/clang/www/make_cxx_dr_status
index cbb0dcc675b2..3a9b18f59cfd 100755
--- a/clang/www/make_cxx_dr_status
+++ b/clang/www/make_cxx_dr_status
@@ -93,7 +93,7 @@ print >> out_file, '''\
 Available in Clang?
   '''
 
-latest_release = 10
+latest_release = 11
 
 def availability(issue):
   status = status_map.get(issue, 'unknown')



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

[PATCH] D84244: [clang] Disable -Wsuggest-override for unittests/

[Hans Wennborg via Phabricator via cfe-commits]

hans added a comment.

> In that case, I've enabled it again using `add_compile_options` instead of 
> `add_definitions`. I've got my finger hovering over the button to revert.

(For reference, the re-commit is 77e0e9e 
.)

Looks good to me so far. We haven't tried doing any full builds yet, but I 
checked for the specific problem I hit yesterday (building 
ClangApplyReplacementsTests) and that's working fine now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84244



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

[PATCH] D83501: [clangd][ObjC] Improve xrefs for protocols and classes

[David Goldman via Phabricator via cfe-commits]

dgoldman updated this revision to Diff 280086.
dgoldman added a comment.

Rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83501

Files:
  clang-tools-extra/clangd/FindTarget.cpp
  clang-tools-extra/clangd/XRefs.cpp
  clang-tools-extra/clangd/index/SymbolCollector.cpp
  clang-tools-extra/clangd/unittests/FindTargetTests.cpp
  clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
  clang-tools-extra/clangd/unittests/XRefsTests.cpp

Index: clang-tools-extra/clangd/unittests/XRefsTests.cpp
===
--- clang-tools-extra/clangd/unittests/XRefsTests.cpp
+++ clang-tools-extra/clangd/unittests/XRefsTests.cpp
@@ -674,7 +674,57 @@
   enum class E { [[A]], B };
   E e = E::A^;
 };
-  )cpp"};
+  )cpp",
+
+  R"objc(
+@protocol Dog;
+@protocol $decl[[Dog]]
+- (void)bark;
+@end
+id getDoggo() {
+  return 0;
+}
+  )objc",
+
+  R"objc(
+@interface Cat
+@end
+@implementation Cat
+@end
+@interface $decl[[Cat]] (Exte^nsion)
+- (void)meow;
+@end
+@implementation $def[[Cat]] (Extension)
+- (void)meow {}
+@end
+  )objc",
+
+  R"objc(
+@class $decl[[Foo]];
+Fo^o * getFoo() {
+  return 0;
+}
+  )objc",
+
+  R"objc(
+@class Foo;
+@interface $decl[[Foo]]
+@end
+Fo^o * getFoo() {
+  return 0;
+}
+  )objc",
+
+  R"objc(
+@class Foo;
+@interface $decl[[Foo]]
+@end
+@implementation $def[[Foo]]
+@end
+Fo^o * getFoo() {
+  return 0;
+}
+  )objc"};
   for (const char *Test : Tests) {
 Annotations T(Test);
 llvm::Optional WantDecl;
@@ -692,6 +742,7 @@
 // FIXME: Auto-completion in a template requires disabling delayed template
 // parsing.
 TU.ExtraArgs.push_back("-fno-delayed-template-parsing");
+TU.ExtraArgs.push_back("-xobjective-c++");
 
 auto AST = TU.build();
 auto Results = locateSymbolAt(AST, T.point());
@@ -709,6 +760,131 @@
   }
 }
 
+TEST(LocateSymbol, AllMulti) {
+  // Ranges in tests:
+  //   $declN is the declaration location
+  //   $defN is the definition location (if absent, symbol has no definition)
+  //
+  // NOTE:
+  //   N starts at 0.
+  //   Due to limitations in clang's Annotations, you can't annotate the same
+  //   text with multiple ranges, e.g. `$def0$def1[[Foo]]` is invalid.
+  struct ExpectedRanges {
+Range WantDecl;
+llvm::Optional WantDef;
+  };
+  const char *Tests[] = {
+  R"objc(
+  @interface $decl0[[Cat]]
+  @end
+  @implementation $def0[[Cat]]
+  @end
+  @interface $decl1[[Ca^t]] (Extension)
+  - (void)meow;
+  @end
+  @implementation $def1[[Cat]] (Extension)
+  - (void)meow {}
+  @end
+)objc",
+
+  R"objc(
+  @interface $decl0[[Cat]]
+  @end
+  @implementation $def0[[Cat]]
+  @end
+  @interface $decl1[[Cat]] (Extension)
+  - (void)meow;
+  @end
+  @implementation $def1[[Ca^t]] (Extension)
+  - (void)meow {}
+  @end
+)objc",
+  };
+  for (const char *Test : Tests) {
+Annotations T(Test);
+std::vector Ranges;
+for (int Idx = 0; true; Idx++) {
+  bool HasDecl = !T.ranges("decl" + std::to_string(Idx)).empty();
+  bool HasDef = !T.ranges("def" + std::to_string(Idx)).empty();
+  if (!HasDecl && !HasDef)
+break;
+  ExpectedRanges Range;
+  if (HasDecl)
+Range.WantDecl = T.range("decl" + std::to_string(Idx));
+  if (HasDef)
+Range.WantDef = T.range("def" + std::to_string(Idx));
+  Ranges.push_back(Range);
+}
+
+TestTU TU;
+TU.Code = std::string(T.code());
+TU.ExtraArgs.push_back("-xobjective-c++");
+
+auto AST = TU.build();
+auto Results = locateSymbolAt(AST, T.point());
+
+ASSERT_THAT(Results, ::testing::SizeIs(Ranges.size())) << Test;
+for (size_t Idx = 0; Idx < Ranges.size(); Idx++) {
+  EXPECT_EQ(Results[Idx].PreferredDeclaration.range, Ranges[Idx].WantDecl)
+  << "($decl" << Idx << ")" << Test;
+  llvm::Optional GotDef;
+  if (Results[Idx].Definition)
+GotDef = Results[Idx].Definition->range;
+  EXPECT_EQ(GotDef, Ranges[Idx].WantDef) << "($def" << Idx << ")" << Test;
+}
+  }
+}
+
+TEST(LocateSymbol, MultipleDeclsWithSameDefinition) {
+  // Ranges in tests:
+  //   $def is the shared definition location
+  //   $declN is one of the declaration locations
+  //
+  // NOTE:
+  //   N starts at 0.
+  const char *Tests[] = {
+  R"objc(
+  @interface $decl0[[Cat]]
+  @end
+  @interface $decl1[[Ca^t]] ()
+  - (void)meow;
+  @end
+  @implementation $def[[Cat]]
+  - (void)meow {}
+  @end
+)objc",
+  };
+  for (c

[PATCH] D84316: [analyzer][NFC] Split CStringChecker to modeling and reporting

[Kristóf Umann via Phabricator via cfe-commits]

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

Yay! This checker has become a major headache as the analyzer grew. Not on a 
RetainCount scale, but on a MallocChecker one. Cheap shots, I know :)

The patch looks great! I have some miscellaneous comments, but nothing blocking.

>> I don't think i understand having `unix.cstring.CStringChecker` as one more 
>> entry in `Checkers.td`. Do you expect there to be a situation when enabling 
>> `CStringModeling` without `CStringChecker` actually makes sense?
> 
> You seem to be right.
>  Enabling only the cstring modeling part does not make much sense without 
> enabling //at least// the CStringChecker to model the cstring manipulation 
> functions - even if the reporting is disabled of such functions.
> 
>> If not, why not keep them agglutinated?
> 
> I wanted to have a separate class for bookkeeping while minimalizing the 
> necessary changes.
>  What do you think would be the best way to organize this separation?
> 
> Few notes:
> 
> - Checkers are implemented in the anonymous namespace, so only the given file 
> has access to them.
> - I wanted to separate the bookkeeping logic from the reporting/function 
> modeling logic in different files.
> - I like the fact that after the change the CStringChecker implements only 
> the evalCall checker callback.
> 
>   Let me know if I misunderstood something.

Mind that that entry does a lot more then give a flag to the user -- It 
generates code for a lot of the checker machinery as well. Since 
CStringModeling still uses the checker callbacks to set up the proper string 
length, it is a necessity. (strong) Checker dependencies are the exact solution 
to the the problem where a checker cannot run without another (as I understand 
it, its not only about making sense, you really cant make CStringChecker work 
without CStringModeling).




Comment at: clang/include/clang/StaticAnalyzer/Checkers/Checkers.td:429
 
 def CStringModeling : Checker<"CStringModeling">,
+  HelpText<"Responsible for essential modeling of cstring lengths. "

What other aspects of c strings needs to be modeled? Is it only length? If so, 
how about we rename the checker to `CStringLengthModeling`?



Comment at: clang/include/clang/StaticAnalyzer/Checkers/Checkers.td:495
   ]>,
-  Dependencies<[CStringModeling]>,
+  Dependencies<[CStringChecker]>,
   Documentation,

I dug around a bit, and found this commit as to why this was needed: 
rGe56167e8f87acf87a9de3d383752e18a738cf056. So this dependency is appropriate.



Comment at: clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp:74
 enum class ConcatFnKind { none = 0, strcat = 1, strlcat = 2 };
-class CStringChecker : public Checker< eval::Call,
- check::PreStmt,
- check::LiveSymbols,
- check::DeadSymbols,
- check::RegionChanges
- > {
+class CStringChecker : public Checker {
   mutable std::unique_ptr BT_Null, BT_Bounds, BT_Overlap,

This is somewhat orthogonal to the patch, but shouldn't precondition violations 
be reported at `preCall`?



Comment at: clang/lib/StaticAnalyzer/Checkers/CStringLength.cpp:175-181
+void ento::registerCStringModeling(CheckerManager &Mgr) {
+  Mgr.registerChecker();
+}
+
+bool ento::shouldRegisterCStringModeling(const CheckerManager &) {
+  return true;
+}

We traditionally put these on the bottom of the file -- I don't think this 
would upset the structure too much :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84316



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

[PATCH] D84382: [PowerPC][Power10] Cleanup p10vector clang test

[Amy Kwan via Phabricator via cfe-commits]

amyk accepted this revision.
amyk added a comment.

LGTM as well.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84382



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

[PATCH] D84048: DR2303: Prefer 'nearer' base classes during template deduction.

[Erich Keane via Phabricator via cfe-commits]

erichkeane updated this revision to Diff 280101.
erichkeane marked 9 inline comments as done.
erichkeane added a comment.

Fix all the things that @rsmith suggested.

Thanks for the feedback, it is looking much better!

I ended up using a MapVector instead of std::map for the 
RecordType*->SmallVector map, since we iterate over it.  While the iteration 
SHOULDN'T matter (it is just the order that bases are added), if there is any 
bug/unconsidered situation, we might diagnose in a non-deterministic manner.  
Let me know if I'm being overly concerned there and I can switch it to std::map 
(DenseMap ends up being a bad choice according to the docs).


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

https://reviews.llvm.org/D84048

Files:
  clang/lib/Sema/SemaTemplateDeduction.cpp
  clang/test/CXX/drs/dr23xx.cpp
  clang/www/cxx_dr_status.html

Index: clang/www/cxx_dr_status.html
===
--- clang/www/cxx_dr_status.html
+++ clang/www/cxx_dr_status.html
@@ -13633,7 +13633,7 @@
 https://wg21.link/cwg2303";>2303
 DRWP
 Partial ordering and recursive variadic inheritance
-Unknown
+Clang 12
   
   
 https://wg21.link/cwg2304";>2304
Index: clang/test/CXX/drs/dr23xx.cpp
===
--- clang/test/CXX/drs/dr23xx.cpp
+++ clang/test/CXX/drs/dr23xx.cpp
@@ -113,3 +113,35 @@
   extern template const int d;
 #endif
 }
+
+#if __cplusplus >= 201103L
+namespace dr2303 { // dr2303: 12
+template 
+struct A;
+template <>
+struct A<> {};
+template 
+struct A : A {};
+struct B : A {};
+struct C : A, A {}; // expected-warning {{direct base 'A' is inaccessible}}
+struct D : A, A {}; // expected-warning {{direct base 'A' is inaccessible}}
+struct E : A {};
+struct F : B, E {};
+
+template 
+void f(const A &) {
+  static_assert(sizeof...(T) == 2, "Should only match A");
+}
+template 
+void f2(const A *);
+
+void g() {
+  f(B{}); // This is no longer ambiguous.
+  B b;
+  f2(&b);
+  f(C{});
+  f(D{});
+  f(F{}); // expected-error {{ambiguous conversion from derived class}}
+}
+} //namespace dr2303
+#endif
Index: clang/lib/Sema/SemaTemplateDeduction.cpp
===
--- clang/lib/Sema/SemaTemplateDeduction.cpp
+++ clang/lib/Sema/SemaTemplateDeduction.cpp
@@ -1201,6 +1201,120 @@
   return false;
 }
 
+///  Attempt to deduce the template arguments by checking the base types
+///  according to (C++ [temp.deduct.call] p4b3.
+///
+/// \param S the semantic analysis object within which we are deducing
+///
+/// \param RecordT the top level record object we are deducing against.
+///
+/// \param TemplateParams the template parameters that we are deducing
+///
+/// \param SpecParam the template specialization parameter type.
+///
+/// \param Info information about the template argument deduction itself
+///
+/// \param Deduced the deduced template arguments
+///
+/// \returns the result of template argument deduction with the bases. "invalid"
+/// means no matches, "success" found a single item, and the
+/// "MiscellaneousDeductionFailure" result happens when the match is ambiguous.
+static Sema::TemplateDeductionResult DeduceTemplateBases(
+Sema &S, const RecordType *RecordT, TemplateParameterList *TemplateParams,
+const TemplateSpecializationType *SpecParam, TemplateDeductionInfo &Info,
+SmallVectorImpl &Deduced) {
+  // C++14 [temp.deduct.call] p4b3:
+  //   If P is a class and P has the form simple-template-id, then the
+  //   transformed A can be a derived class of the deduced A. Likewise if
+  //   P is a pointer to a class of the form simple-template-id, the
+  //   transformed A can be a pointer to a derived class pointed to by the
+  //   deduced A. However, if there is a class C that is a (direct or
+  //   indirect) base class of D and derived (directly or indirectly) from a
+  //   class B and that would be a valid deduced A, the deduced A cannot be
+  //   B or pointer to B, respectively.
+  //
+  //   These alternatives are considered only if type deduction would
+  //   otherwise fail. If they yield more than one possible deduced A, the
+  //   type deduction fails.
+
+  // Use a breadth-first search through the bases to collect the set of
+  // successful matches. Visited contains the set of nodes we have already
+  // visited, while ToVisit is our stack of records that we still need to
+  // visit.  Matches contains a list of matches that have yet to be
+  // disqualified.
+  llvm::SmallPtrSet Visited;
+  SmallVector ToVisit;
+  // We iterate over this later, so we have to use MapVector to ensure
+  // determinism.
+  llvm::MapVector>
+  Matches;
+
+  auto AddBases = [&Visited, &ToVisit](const RecordType *RT) {
+CXXRecordDecl *RD = cast(RT->getDecl());
+for (const auto &Base : RD->bases()) {
+  assert(Base.getType()->isRecordType() &&
+ "Base class that isn't a record?");
+  

[PATCH] D84364: [CUDA][HIP] Defer all diagnostics for host device functions

[Yaxun Liu via Phabricator via cfe-commits]

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

added lit test for SFINAE


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

https://reviews.llvm.org/D84364

Files:
  clang/include/clang/Basic/LangOptions.def
  clang/include/clang/Driver/Options.td
  clang/include/clang/Sema/Sema.h
  clang/lib/Driver/ToolChains/Cuda.cpp
  clang/lib/Driver/ToolChains/HIP.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/Sema/AnalysisBasedWarnings.cpp
  clang/lib/Sema/Sema.cpp
  clang/lib/Sema/SemaAttr.cpp
  clang/lib/Sema/SemaCUDA.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaExprObjC.cpp
  clang/lib/Sema/SemaOpenMP.cpp
  clang/lib/Sema/SemaSYCL.cpp
  clang/lib/Sema/SemaStmt.cpp
  clang/lib/Sema/SemaStmtAsm.cpp
  clang/lib/Sema/SemaTemplateInstantiate.cpp
  clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
  clang/lib/Sema/SemaTemplateVariadic.cpp
  clang/lib/Sema/SemaType.cpp
  clang/test/SemaCUDA/deferred-all.cu

Index: clang/test/SemaCUDA/deferred-all.cu
===
--- /dev/null
+++ clang/test/SemaCUDA/deferred-all.cu
@@ -0,0 +1,66 @@
+// RUN: %clang_cc1 -fcuda-is-device -fsyntax-only -verify=dev,com %s \
+// RUN:   -std=c++11 -fgpu-defer-diag
+// RUN: %clang_cc1 -fsyntax-only -verify=host,com %s \
+// RUN:   -std=c++11 -fgpu-defer-diag
+
+#include "Inputs/cuda.h"
+
+__device__ void callee(int);
+__host__ void callee(float); // host-note 2{{candidate function}}
+__host__ void callee(double); // host-note 2{{candidate function}}
+
+// Check no diagnostics for this function since it is never
+// called.
+
+inline __host__ __device__ void hdf_not_called() {
+  callee(1);
+  bad_line
+}
+
+// When emitted on device, there is syntax error.
+// When emitted on host, there is ambiguity and syntax error.
+  
+inline __host__ __device__ void hdf_called() {
+  callee(1); // host-error {{call to 'callee' is ambiguous}}
+  bad_line // com-error {{use of undeclared identifier 'bad_line'}}
+}
+
+// This is similar to the above but is always emitted on
+// both sides.
+
+__host__ __device__ void hdf_always_emitted() {
+  callee(1); // host-error {{call to 'callee' is ambiguous}}
+  bad_line // com-error {{use of undeclared identifier 'bad_line'}}
+}
+
+void hf() {
+ hdf_called(); // host-note {{called by 'hf'}}
+}
+ 
+__device__ void df() {
+ hdf_called(); // dev-note {{called by 'df'}}
+}
+
+struct A { int x; typedef int type; };
+struct B { int x; };
+
+// This function is defined for A only by SFINAE.
+// Calling it with A should succeed, with B should fail.
+// The error should not be deferred since it happens in
+// file scope.
+
+template
+__host__ __device__ void sfinae(T t) { // com-note {{candidate template ignored: substitution failure [with T = B]}}
+  t.x = 1;
+}
+
+void test_sfinae() {
+  sfinae(A());
+  sfinae(B()); // com-error{{no matching function for call to 'sfinae'}}
+}
+
+// If a syntax error causes a function not declared, it cannot
+// be deferred.
+
+inline __host__ __device__ void bad_func() { // com-note {{to match this '{'}}
+// com-error {{expected '}'}}
Index: clang/lib/Sema/SemaType.cpp
===
--- clang/lib/Sema/SemaType.cpp
+++ clang/lib/Sema/SemaType.cpp
@@ -4087,7 +4087,8 @@
 
 /// Creates a fix-it to insert a C-style nullability keyword at \p pointerLoc,
 /// taking into account whitespace before and after.
-static void fixItNullability(Sema &S, DiagnosticBuilder &Diag,
+template 
+static void fixItNullability(Sema &S, DiagBuilderT &Diag,
  SourceLocation PointerLoc,
  NullabilityKind Nullability) {
   assert(PointerLoc.isValid());
Index: clang/lib/Sema/SemaTemplateVariadic.cpp
===
--- clang/lib/Sema/SemaTemplateVariadic.cpp
+++ clang/lib/Sema/SemaTemplateVariadic.cpp
@@ -368,8 +368,8 @@
   Locations.push_back(Unexpanded[I].second);
   }
 
-  DiagnosticBuilder DB = Diag(Loc, diag::err_unexpanded_parameter_pack)
- << (int)UPPC << (int)Names.size();
+  auto DB = Diag(Loc, diag::err_unexpanded_parameter_pack)
+<< (int)UPPC << (int)Names.size();
   for (size_t I = 0, E = std::min(Names.size(), (size_t)2); I != E; ++I)
 DB << Names[I];
 
Index: clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
===
--- clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -5999,7 +5999,7 @@
 if (!Result) {
   if (isa(D)) {
 // UsingShadowDecls can instantiate to nothing because of using hiding.
-  } else if (Diags.hasUncompilableErrorOccurred()) {
+  } else if (hasUncompilableErrorOccurred()) {
 // We've already complained about some ill-formed code, so most likely
 // this declaration failed to instantiate. There's no point in
 // complaini

[PATCH] D80858: [CUDA][HIP] Support accessing static device variable in host code

[Michael Liao via Phabricator via cfe-commits]

hliao requested changes to this revision.
hliao added a comment.
This revision now requires changes to proceed.

I don't that's proper way to support file-scope static device variables. As we 
discuss APIs like cudaMemcpyToSymol, that's a runtime API instead of driver 
API. The later needs to specify the module (or code object) id in addition to a 
symbol name and won't have the conflict issues. For the runtime API, all named 
device variables (static or not) are identified at the host side by their host 
stub variables. That stub variable is used to register the corresponding device 
variables associated with a module id to unique identify that variables across 
all TUs. As long as we look up device variables using their host stub variable 
pointers, they are uniquely identified already. The runtime implementation 
needs to find the module id and the variable symbol from the pointer of its 
host stub variable. It's not the compiler job to fabricate name uniquely across 
TUs.


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

https://reviews.llvm.org/D80858



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

[PATCH] D83915: [PowerPC] Remove QPX/A2Q BGQ/BGP support

[Jinsong Ji via Phabricator via cfe-commits]

jsji added a comment.

Ping ... Any other comments? Thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83915



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

[PATCH] D84405: CodeGen: Improve generated IR for __builtin_mul_overflow(uint, uint, int)

[Tom Stellard via Phabricator via cfe-commits]

tstellar created this revision.
tstellar added reviewers: vsk, efriedma.
Herald added a project: clang.

Add a special case for handling __builtin_mul_overflow with unsigned
inputs and a signed output to avoid emitting the __muloti4 library
call on x86_64.  __muloti4 is not implemented in libgcc, so avoiding
this call fixes compilation of some programs that call
__builtin_mul_overflow with these arguments.

For example, this fixes the build of cpio with clang, which includes code from
gnulib that calls __builtin_mul_overflow with these argument types.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D84405

Files:
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/test/CodeGen/builtins-overflow.c


Index: clang/test/CodeGen/builtins-overflow.c
===
--- clang/test/CodeGen/builtins-overflow.c
+++ clang/test/CodeGen/builtins-overflow.c
@@ -111,6 +111,21 @@
   return r;
 }
 
+int test_mul_overflow_uint_uint_int(unsigned x, unsigned y) {
+  // CHECK-LABEL: define {{(dso_local )?}}i32 @test_mul_overflow_uint_uint_int
+  // CHECK: [[S:%.+]] = call { i32, i1 } @llvm.umul.with.overflow.i32(i32 
%{{.+}}, i32 %{{.+}})
+  // CHECK-DAG: [[Q:%.+]] = extractvalue { i32, i1 } [[S]], 0
+  // CHECK-DAG: [[C:%.+]] = extractvalue { i32, i1 } [[S]], 1
+  // CHECK: [[C1:%.+]] = icmp ugt i32 [[Q]], 2147483647
+  // CHECK: [[C2:%.+]] = or i1 [[C]], [[C1]]
+  // CHECK: store i32 [[Q]], i32*
+  // CHECK: br i1 [[C2]]
+  int r;
+  if (__builtin_mul_overflow(x, y, &r))
+overflowed();
+  return r;
+}
+
 int test_mul_overflow_int_int_int(int x, int y) {
   // CHECK-LABEL: define {{(dso_local )?}}i32 @test_mul_overflow_int_int_int
   // CHECK-NOT: ext
Index: clang/lib/CodeGen/CGBuiltin.cpp
===
--- clang/lib/CodeGen/CGBuiltin.cpp
+++ clang/lib/CodeGen/CGBuiltin.cpp
@@ -1408,6 +1408,47 @@
   return RValue::get(BufAddr.getPointer());
 }
 
+static bool isSpecialUnsignedMultiplySignedResult(
+unsigned BuiltinID, WidthAndSignedness Op1Info, WidthAndSignedness Op2Info,
+WidthAndSignedness ResultInfo) {
+  return BuiltinID == Builtin::BI__builtin_mul_overflow &&
+ Op1Info.Width == Op2Info.Width && Op2Info.Width == ResultInfo.Width &&
+ !Op1Info.Signed && !Op2Info.Signed && ResultInfo.Signed;
+}
+
+static RValue EmitCheckedUnsignedMultiplySignedResult(
+CodeGenFunction &CGF, const clang::Expr *Op1, WidthAndSignedness Op1Info,
+const clang::Expr *Op2, WidthAndSignedness Op2Info,
+const clang::Expr *ResultArg, QualType ResultQTy,
+WidthAndSignedness ResultInfo) {
+  assert(isSpecialUnsignedMultiplySignedResult(
+ Builtin::BI__builtin_mul_overflow, Op1Info, Op2Info, ResultInfo) 
&&
+ "Cannot specialize this multiply");
+
+  llvm::Value *V1 = CGF.EmitScalarExpr(Op1);
+  llvm::Value *V2 = CGF.EmitScalarExpr(Op2);
+
+  llvm::Value *HasOverflow;
+  llvm::Value *Result = EmitOverflowIntrinsic(
+  CGF, llvm::Intrinsic::umul_with_overflow, V1, V2, HasOverflow);
+
+  // The intrinsic call will detect overflow when the value is > UINT_MAX,
+  // however, since the original builtin had a signed result, we need to report
+  // an overflow when the result is greater than INT_MAX.
+  auto IntMax = llvm::APInt::getSignedMaxValue(ResultInfo.Width);
+  llvm::Value *IntMaxValue = llvm::ConstantInt::get(Result->getType(), IntMax);
+
+  llvm::Value *IntMaxOverflow = CGF.Builder.CreateICmpUGT(Result, IntMaxValue);
+  HasOverflow = CGF.Builder.CreateOr(HasOverflow, IntMaxOverflow);
+
+  bool isVolatile =
+  ResultArg->getType()->getPointeeType().isVolatileQualified();
+  Address ResultPtr = CGF.EmitPointerWithAlignment(ResultArg);
+  CGF.Builder.CreateStore(CGF.EmitToMemory(Result, ResultQTy), ResultPtr,
+  isVolatile);
+  return RValue::get(HasOverflow);
+}
+
 /// Determine if a binop is a checked mixed-sign multiply we can specialize.
 static bool isSpecialMixedSignMultiply(unsigned BuiltinID,
WidthAndSignedness Op1Info,
@@ -3484,6 +3525,12 @@
   RightInfo, ResultArg, ResultQTy,
   ResultInfo);
 
+if (isSpecialUnsignedMultiplySignedResult(BuiltinID, LeftInfo, RightInfo,
+  ResultInfo))
+  return EmitCheckedUnsignedMultiplySignedResult(
+  *this, LeftArg, LeftInfo, RightArg, RightInfo, ResultArg, ResultQTy,
+  ResultInfo);
+
 WidthAndSignedness EncompassingInfo =
 EncompassingIntegerType({LeftInfo, RightInfo, ResultInfo});
 
@@ -3507,6 +3554,8 @@
 : llvm::Intrinsic::usub_with_overflow;
   break;
 case Builtin::BI__builtin_mul_overflow:
+  // It's OK to emit umul when the result type is signed, as long as the
+  // input types are both unsigned.
   IntrinsicId = EncompassingInfo.Signed
 

[PATCH] D84405: CodeGen: Improve generated IR for __builtin_mul_overflow(uint, uint, int)

[Tom Stellard via Phabricator via cfe-commits]

tstellar updated this revision to Diff 280104.
tstellar added a comment.

Remove stray comment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84405

Files:
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/test/CodeGen/builtins-overflow.c


Index: clang/test/CodeGen/builtins-overflow.c
===
--- clang/test/CodeGen/builtins-overflow.c
+++ clang/test/CodeGen/builtins-overflow.c
@@ -111,6 +111,21 @@
   return r;
 }
 
+int test_mul_overflow_uint_uint_int(unsigned x, unsigned y) {
+  // CHECK-LABEL: define {{(dso_local )?}}i32 @test_mul_overflow_uint_uint_int
+  // CHECK: [[S:%.+]] = call { i32, i1 } @llvm.umul.with.overflow.i32(i32 
%{{.+}}, i32 %{{.+}})
+  // CHECK-DAG: [[Q:%.+]] = extractvalue { i32, i1 } [[S]], 0
+  // CHECK-DAG: [[C:%.+]] = extractvalue { i32, i1 } [[S]], 1
+  // CHECK: [[C1:%.+]] = icmp ugt i32 [[Q]], 2147483647
+  // CHECK: [[C2:%.+]] = or i1 [[C]], [[C1]]
+  // CHECK: store i32 [[Q]], i32*
+  // CHECK: br i1 [[C2]]
+  int r;
+  if (__builtin_mul_overflow(x, y, &r))
+overflowed();
+  return r;
+}
+
 int test_mul_overflow_int_int_int(int x, int y) {
   // CHECK-LABEL: define {{(dso_local )?}}i32 @test_mul_overflow_int_int_int
   // CHECK-NOT: ext
Index: clang/lib/CodeGen/CGBuiltin.cpp
===
--- clang/lib/CodeGen/CGBuiltin.cpp
+++ clang/lib/CodeGen/CGBuiltin.cpp
@@ -1408,6 +1408,47 @@
   return RValue::get(BufAddr.getPointer());
 }
 
+static bool isSpecialUnsignedMultiplySignedResult(
+unsigned BuiltinID, WidthAndSignedness Op1Info, WidthAndSignedness Op2Info,
+WidthAndSignedness ResultInfo) {
+  return BuiltinID == Builtin::BI__builtin_mul_overflow &&
+ Op1Info.Width == Op2Info.Width && Op2Info.Width == ResultInfo.Width &&
+ !Op1Info.Signed && !Op2Info.Signed && ResultInfo.Signed;
+}
+
+static RValue EmitCheckedUnsignedMultiplySignedResult(
+CodeGenFunction &CGF, const clang::Expr *Op1, WidthAndSignedness Op1Info,
+const clang::Expr *Op2, WidthAndSignedness Op2Info,
+const clang::Expr *ResultArg, QualType ResultQTy,
+WidthAndSignedness ResultInfo) {
+  assert(isSpecialUnsignedMultiplySignedResult(
+ Builtin::BI__builtin_mul_overflow, Op1Info, Op2Info, ResultInfo) 
&&
+ "Cannot specialize this multiply");
+
+  llvm::Value *V1 = CGF.EmitScalarExpr(Op1);
+  llvm::Value *V2 = CGF.EmitScalarExpr(Op2);
+
+  llvm::Value *HasOverflow;
+  llvm::Value *Result = EmitOverflowIntrinsic(
+  CGF, llvm::Intrinsic::umul_with_overflow, V1, V2, HasOverflow);
+
+  // The intrinsic call will detect overflow when the value is > UINT_MAX,
+  // however, since the original builtin had a signed result, we need to report
+  // an overflow when the result is greater than INT_MAX.
+  auto IntMax = llvm::APInt::getSignedMaxValue(ResultInfo.Width);
+  llvm::Value *IntMaxValue = llvm::ConstantInt::get(Result->getType(), IntMax);
+
+  llvm::Value *IntMaxOverflow = CGF.Builder.CreateICmpUGT(Result, IntMaxValue);
+  HasOverflow = CGF.Builder.CreateOr(HasOverflow, IntMaxOverflow);
+
+  bool isVolatile =
+  ResultArg->getType()->getPointeeType().isVolatileQualified();
+  Address ResultPtr = CGF.EmitPointerWithAlignment(ResultArg);
+  CGF.Builder.CreateStore(CGF.EmitToMemory(Result, ResultQTy), ResultPtr,
+  isVolatile);
+  return RValue::get(HasOverflow);
+}
+
 /// Determine if a binop is a checked mixed-sign multiply we can specialize.
 static bool isSpecialMixedSignMultiply(unsigned BuiltinID,
WidthAndSignedness Op1Info,
@@ -3484,6 +3525,12 @@
   RightInfo, ResultArg, ResultQTy,
   ResultInfo);
 
+if (isSpecialUnsignedMultiplySignedResult(BuiltinID, LeftInfo, RightInfo,
+  ResultInfo))
+  return EmitCheckedUnsignedMultiplySignedResult(
+  *this, LeftArg, LeftInfo, RightArg, RightInfo, ResultArg, ResultQTy,
+  ResultInfo);
+
 WidthAndSignedness EncompassingInfo =
 EncompassingIntegerType({LeftInfo, RightInfo, ResultInfo});
 


Index: clang/test/CodeGen/builtins-overflow.c
===
--- clang/test/CodeGen/builtins-overflow.c
+++ clang/test/CodeGen/builtins-overflow.c
@@ -111,6 +111,21 @@
   return r;
 }
 
+int test_mul_overflow_uint_uint_int(unsigned x, unsigned y) {
+  // CHECK-LABEL: define {{(dso_local )?}}i32 @test_mul_overflow_uint_uint_int
+  // CHECK: [[S:%.+]] = call { i32, i1 } @llvm.umul.with.overflow.i32(i32 %{{.+}}, i32 %{{.+}})
+  // CHECK-DAG: [[Q:%.+]] = extractvalue { i32, i1 } [[S]], 0
+  // CHECK-DAG: [[C:%.+]] = extractvalue { i32, i1 } [[S]], 1
+  // CHECK: [[C1:%.+]] = icmp ugt i32 [[Q]], 2147483647
+  // CHECK: [[C2:%.+]] = or i1 [[C]], [[C1]]
+  // CHECK:

[PATCH] D84029: [clang][Driver] Default to /usr/bin/ld on Solaris

[Rainer Orth via Phabricator via cfe-commits]

ro updated this revision to Diff 280108.
ro added a reviewer: MaskRay.
ro added a comment.

Rebased after D83015 .


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84029

Files:
  clang/lib/Driver/ToolChain.cpp
  clang/lib/Driver/ToolChains/Solaris.h


Index: clang/lib/Driver/ToolChains/Solaris.h
===
--- clang/lib/Driver/ToolChains/Solaris.h
+++ clang/lib/Driver/ToolChains/Solaris.h
@@ -65,6 +65,11 @@
   SanitizerMask getSupportedSanitizers() const override;
   unsigned GetDefaultDwarfVersion() const override { return 2; }
 
+  const char *getDefaultLinker() const override {
+// clang currently uses Solaris ld-only options.
+return "/usr/bin/ld";
+  }
+
 protected:
   Tool *buildAssembler() const override;
   Tool *buildLinker() const override;
Index: clang/lib/Driver/ToolChain.cpp
===
--- clang/lib/Driver/ToolChain.cpp
+++ clang/lib/Driver/ToolChain.cpp
@@ -568,8 +568,13 @@
   }
   // If we're passed -fuse-ld= with no argument, or with the argument ld,
   // then use whatever the default system linker is.
-  if (UseLinker.empty() || UseLinker == "ld")
-return GetProgramPath(getDefaultLinker());
+  if (UseLinker.empty() || UseLinker == "ld") {
+const char *DefaultLinker = getDefaultLinker();
+if (!llvm::sys::path::is_absolute(DefaultLinker))
+  return GetProgramPath(DefaultLinker);
+else
+  return std::string(DefaultLinker);
+  }
 
   // Extending -fuse-ld= to an absolute or relative path is unexpected. 
Checking
   // for the linker flavor is brittle. In addition, prepending "ld." or "ld64."


Index: clang/lib/Driver/ToolChains/Solaris.h
===
--- clang/lib/Driver/ToolChains/Solaris.h
+++ clang/lib/Driver/ToolChains/Solaris.h
@@ -65,6 +65,11 @@
   SanitizerMask getSupportedSanitizers() const override;
   unsigned GetDefaultDwarfVersion() const override { return 2; }
 
+  const char *getDefaultLinker() const override {
+// clang currently uses Solaris ld-only options.
+return "/usr/bin/ld";
+  }
+
 protected:
   Tool *buildAssembler() const override;
   Tool *buildLinker() const override;
Index: clang/lib/Driver/ToolChain.cpp
===
--- clang/lib/Driver/ToolChain.cpp
+++ clang/lib/Driver/ToolChain.cpp
@@ -568,8 +568,13 @@
   }
   // If we're passed -fuse-ld= with no argument, or with the argument ld,
   // then use whatever the default system linker is.
-  if (UseLinker.empty() || UseLinker == "ld")
-return GetProgramPath(getDefaultLinker());
+  if (UseLinker.empty() || UseLinker == "ld") {
+const char *DefaultLinker = getDefaultLinker();
+if (!llvm::sys::path::is_absolute(DefaultLinker))
+  return GetProgramPath(DefaultLinker);
+else
+  return std::string(DefaultLinker);
+  }
 
   // Extending -fuse-ld= to an absolute or relative path is unexpected. Checking
   // for the linker flavor is brittle. In addition, prepending "ld." or "ld64."
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D84409: [libTooling] Add an `EditGenerator` that applies a rule throughout a bound node.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel created this revision.
ymandel added a reviewer: gribozavr2.
Herald added a project: clang.

The new combinator, `rewriteDescendants`, applies a rewrite rule to all
descendants of a specified bound node.  That rewrite rule can refer to nodes
bound by the parent, both in the matcher and in the edits.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D84409

Files:
  clang/include/clang/Tooling/Transformer/RewriteRule.h
  clang/lib/Tooling/Transformer/RewriteRule.cpp
  clang/lib/Tooling/Transformer/Transformer.cpp
  clang/unittests/Tooling/TransformerTest.cpp

Index: clang/unittests/Tooling/TransformerTest.cpp
===
--- clang/unittests/Tooling/TransformerTest.cpp
+++ clang/unittests/Tooling/TransformerTest.cpp
@@ -114,7 +114,9 @@
   if (C) {
 Changes.push_back(std::move(*C));
   } else {
-consumeError(C.takeError());
+// FIXME: stash this error rather then printing.
+llvm::errs() << "Error generating changes: "
+ << llvm::toString(C.takeError()) << "\n";
 ++ErrorCount;
   }
 };
@@ -414,6 +416,71 @@
Input, Expected);
 }
 
+// Rewrite various Stmts inside a Decl.
+TEST_F(TransformerTest, RewriteDescendantsDeclChangeStmt) {
+  std::string Input =
+  "int f(int x) { int y = x; { int z = x * x; } return x; }";
+  std::string Expected =
+  "int f(int x) { int y = 3; { int z = 3 * 3; } return 3; }";
+  auto InlineX =
+  makeRule(declRefExpr(to(varDecl(hasName("x", changeTo(cat("3")));
+  testRule(makeRule(functionDecl(hasName("f")).bind("fun"),
+rewriteDescendants("fun", InlineX)),
+   Input, Expected);
+}
+
+// Rewrite various TypeLocs inside a Decl.
+TEST_F(TransformerTest, RewriteDescendantsDeclChangeTypeLoc) {
+  std::string Input = "int f(int *x) { return *x; }";
+  std::string Expected = "char f(char *x) { return *x; }";
+  auto IntToChar = makeRule(typeLoc(loc(qualType(isInteger(), builtinType(,
+changeTo(cat("char")));
+  testRule(makeRule(functionDecl(hasName("f")).bind("fun"),
+rewriteDescendants("fun", IntToChar)),
+   Input, Expected);
+}
+
+TEST_F(TransformerTest, RewriteDescendantsStmt) {
+  std::string Input =
+  "int f(int x) { int y = x; { int z = x * x; } return x; }";
+  std::string Expected =
+  "int f(int x) { int y = 3; { int z = 3 * 3; } return 3; }";
+  auto InlineX =
+  makeRule(declRefExpr(to(varDecl(hasName("x", changeTo(cat("3")));
+  testRule(makeRule(functionDecl(hasName("f"), hasBody(stmt().bind("body"))),
+rewriteDescendants("body", InlineX)),
+   Input, Expected);
+}
+
+TEST_F(TransformerTest, RewriteDescendantsTypeLoc) {
+  std::string Input = "int f(int *x) { return *x; }";
+  std::string Expected = "int f(char *x) { return *x; }";
+  auto IntToChar =
+  makeRule(typeLoc(loc(qualType(isInteger(), builtinType(.bind("loc"),
+   changeTo(cat("char")));
+  testRule(
+  makeRule(functionDecl(hasName("f"),
+hasParameter(0, varDecl(hasTypeLoc(
+typeLoc().bind("parmType"),
+   rewriteDescendants("parmType", IntToChar)),
+  Input, Expected);
+}
+
+TEST_F(TransformerTest, RewriteDescendantsReferToParentBinding) {
+  std::string Input =
+  "int f(int p) { int y = p; { int z = p * p; } return p; }";
+  std::string Expected =
+  "int f(int p) { int y = 3; { int z = 3 * 3; } return 3; }";
+  std::string VarId = "var";
+  auto InlineVar = makeRule(declRefExpr(to(varDecl(equalsBoundNode(VarId,
+changeTo(cat("3")));
+  testRule(makeRule(functionDecl(hasName("f"),
+ hasParameter(0, varDecl().bind(VarId)))
+.bind("fun"),
+rewriteDescendants("fun", InlineVar)),
+   Input, Expected);
+}
+
 TEST_F(TransformerTest, InsertBeforeEdit) {
   std::string Input = R"cc(
 int f() {
@@ -1064,4 +1131,35 @@
   EXPECT_EQ(format(*UpdatedCode), format(R"cc(#include "input.h"
 ;)cc"));
 }
+
+TEST_F(TransformerTest, RewriteDescendantsUnboundNode) {
+  std::string Input =
+  "int f(int x) { int y = x; { int z = x * x; } return x; }";
+  auto InlineX =
+  makeRule(declRefExpr(to(varDecl(hasName("x", changeTo(cat("3")));
+  Transformer T(makeRule(functionDecl(hasName("f")),
+ rewriteDescendants("UNBOUND", InlineX)),
+consumer());
+  T.registerMatchers(&MatchFinder);
+  EXPECT_FALSE(rewrite(Input));
+  EXPECT_THAT(Changes, IsEmpty());
+  EXPECT_EQ(ErrorCount, 1);
+}
+
+TEST_F(TransformerTest, RewriteDescendantsInvalidNodeType) {
+  std::string Input =
+  "int f(int x) { int y = x; { int z = x * x; } return x; }";
+  auto IntToChar =
+  makeRule(qualType(isInteger(), builtinType()),

[PATCH] D83325: [Sema] Be more thorough when unpacking the AS-qualified pointee for a pointer conversion.

[Sven van Haastregt via Phabricator via cfe-commits]

svenvh added a comment.

Wondering if stripping the sugar is the right thing to do here, because it 
means we don't have any sugar on the resulting type if it has gone through the 
SCS and pointer type reconstruction?

> I'm unsure if this is currently broken upstream without other patches to 
> exploit the behavior,

I'm not sure either; perhaps @rjmccall has some insights.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83325



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

[PATCH] D80858: [CUDA][HIP] Support accessing static device variable in host code

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added a comment.

In D80858#2169295 , @hliao wrote:

> I don't that's proper way to support file-scope static device variables. As 
> we discuss APIs like cudaMemcpyToSymol, that's a runtime API instead of 
> driver API. The later needs to specify the module (or code object) id in 
> addition to a symbol name and won't have the conflict issues. For the runtime 
> API, all named device variables (static or not) are identified at the host 
> side by their host stub variables. That stub variable is used to register the 
> corresponding device variables associated with a module id to unique identify 
> that variables across all TUs. As long as we look up device variables using 
> their host stub variable pointers, they are uniquely identified already. The 
> runtime implementation needs to find the module id and the variable symbol 
> from the pointer of its host stub variable. It's not the compiler job to 
> fabricate name uniquely across TUs.


The problem is that even though the static variable is registered through 
`__hipRigisterVariable`, the runtime still relies on looking up symbol name to 
get the address of the device variable. That's why we need to externalize the 
static variable.


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

https://reviews.llvm.org/D80858



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

[PATCH] D84412: [clang][Driver] Don't hardcode --as-needed/--no-as-needed on Illumos

[Rainer Orth via Phabricator via cfe-commits]

ro created this revision.
ro added a reviewer: clang.
ro added a project: clang.
Herald added subscribers: fedor.sergeev, jyknight.

`ninja check-all` currently fails on Illumos:

  [84/716] Generating default/Asan-i386-inline-Test
  FAILED: projects/compiler-rt/lib/asan/tests/default/Asan-i386-inline-Test
  cd /var/llvm/dist-amd64-release/projects/compiler-rt/lib/asan/tests && 
/var/llvm/dist-amd64-release/./bin/clang 
ASAN_INST_TEST_OBJECTS.gtest-all.cc.i386-inline.o 
ASAN_INST_TEST_OBJECTS.asan_globals_test.cpp.i386-inline.o 
ASAN_INST_TEST_OBJECTS.asan_interface_test.cpp.i386-inline.o 
ASAN_INST_TEST_OBJECTS.asan_internal_interface_test.cpp.i386-inline.o 
ASAN_INST_TEST_OBJECTS.asan_test.cpp.i386-inline.o 
ASAN_INST_TEST_OBJECTS.asan_oob_test.cpp.i386-inline.o 
ASAN_INST_TEST_OBJECTS.asan_mem_test.cpp.i386-inline.o 
ASAN_INST_TEST_OBJECTS.asan_str_test.cpp.i386-inline.o 
ASAN_INST_TEST_OBJECTS.asan_test_main.cpp.i386-inline.o -o 
/var/llvm/dist-amd64-release/projects/compiler-rt/lib/asan/tests/default/./Asan-i386-inline-Test
 -g --driver-mode=g++ -fsanitize=address -m32
  ld: fatal: unrecognized option '--no-as-needed'
  ld: fatal: use the -z help option for usage information
  clang-11: error: linker command failed with exit code 1 (use -v to see 
invocation)

`clang` unconditionally passes `--as-needed`/`--no-as-needed` to the linker.  
This works
on Solaris 11.[34] which added a couple of option aliases to the native linker 
to improve
compatibility with GNU `ld`. Illumos `ld` didn't do this, so one needs to use 
the corresponding
native options `-z ignore`/`-z record` instead.

Because this works on both Solaris and Illumos, the current patch always passes 
the
native options on Solaris.  This isn't fully correct, however: when using GNU 
`ld` on 
Solaris (not yet supported; I'm working on that), one still needs `--as-needed` 
instead.

I'm hardcoding this decision because a generic detection via a `cmake` test is 
hard:
many systems have their own implementation of `getDefaultLinker` and `cmake` 
would
have to duplicate the information encoded there.  Besides, it would still break 
when
`-fuse-ld` is used.

Tested on `amd64-pc-solaris2.11` (Solaris 11.4 and OpenIndiana 2020.04), 
`sparcv9-sun-solaris2.11`, and
`x86_64-pc-linux-gnu`.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D84412

Files:
  clang/lib/Driver/ToolChains/CommonArgs.cpp


Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -630,6 +630,14 @@
   return false;
 }
 
+static const char *getAsNeededOption(const ToolChain &TC, bool ignore) {
+  // FIXME: Need to handle GNU ld on Solaris.
+  if (TC.getTriple().isOSSolaris())
+return ignore ? "-zignore" : "-zrecord";
+  else
+return ignore ? "--as-needed" : "--no-as-needed";
+}
+
 void tools::linkSanitizerRuntimeDeps(const ToolChain &TC,
  ArgStringList &CmdArgs) {
   // Fuchsia never needs these.  Any sanitizer runtimes with system
@@ -639,7 +647,7 @@
 
   // Force linking against the system libraries sanitizers depends on
   // (see PR15823 why this is necessary).
-  CmdArgs.push_back("--no-as-needed");
+  CmdArgs.push_back(getAsNeededOption(TC, false));
   // There's no libpthread or librt on RTEMS & Android.
   if (TC.getTriple().getOS() != llvm::Triple::RTEMS &&
   !TC.getTriple().isAndroid()) {
@@ -833,7 +841,7 @@
 }
 
 void tools::linkXRayRuntimeDeps(const ToolChain &TC, ArgStringList &CmdArgs) {
-  CmdArgs.push_back("--no-as-needed");
+  CmdArgs.push_back(getAsNeededOption(TC, false));
   CmdArgs.push_back("-lpthread");
   if (!TC.getTriple().isOSOpenBSD())
 CmdArgs.push_back("-lrt");
@@ -1258,7 +1266,7 @@
   bool AsNeeded = LGT == LibGccType::UnspecifiedLibGcc &&
   !TC.getTriple().isAndroid() && !TC.getTriple().isOSCygMing();
   if (AsNeeded)
-CmdArgs.push_back("--as-needed");
+CmdArgs.push_back(getAsNeededOption(TC, true));
 
   switch (UNW) {
   case ToolChain::UNW_None:
@@ -1286,7 +1294,7 @@
   }
 
   if (AsNeeded)
-CmdArgs.push_back("--no-as-needed");
+CmdArgs.push_back(getAsNeededOption(TC, false));
 }
 
 static void AddLibgcc(const ToolChain &TC, const Driver &D,


Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -630,6 +630,14 @@
   return false;
 }
 
+static const char *getAsNeededOption(const ToolChain &TC, bool ignore) {
+  // FIXME: Need to handle GNU ld on Solaris.
+  if (TC.getTriple().isOSSolaris())
+return ignore ? "-zignore" : "-zrecord";
+  else
+return ignore ? "--as-needed" : "--no-as-needed";
+}
+
 void tools::linkSanitizerRuntimeDeps(const ToolChain &TC,
  ArgStringList &CmdArgs) {
   // Fuc

[PATCH] D84414: [RISCV] Support Shadow Call Stack

[Z. Zheng via Phabricator via cfe-commits]

zzheng created this revision.
zzheng added reviewers: apazos, lenary, asb.
Herald added subscribers: cfe-commits, aaron.ballman, evandro, luismarques, 
sameer.abuasal, pzheng, s.egerton, Jim, benna, psnobl, jocewei, PkmX, rkruppe, 
the_o, brucehoult, MartinMosbeck, rogfer01, edward-jones, MaskRay, jrtc27, 
shiva0217, kito-cheng, niosHD, sabuasal, simoncook, johnrusso, rbar, hiraditya, 
kristof.beyls.
Herald added projects: clang, LLVM.

Currenlty mimics AArch64's implementation. Assume x18 is used as pointer to
shadow call stack. User shall pass flags:

  "-fsanitize=shadow-call-stack -ffixed-x18"

Runtime supported is needed to setup x18.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D84414

Files:
  clang/lib/Driver/SanitizerArgs.cpp
  clang/lib/Driver/ToolChain.cpp
  clang/test/CodeGen/shadowcallstack-attr.c
  clang/test/Driver/sanitizer-ld.c
  llvm/lib/Target/RISCV/RISCVFrameLowering.cpp
  llvm/test/CodeGen/RISCV/shadowcallstack.ll

Index: llvm/test/CodeGen/RISCV/shadowcallstack.ll
===
--- /dev/null
+++ llvm/test/CodeGen/RISCV/shadowcallstack.ll
@@ -0,0 +1,88 @@
+; RUN: llc -verify-machineinstrs -o - %s -mtriple=riscv32-linux-gnu -mattr=+reserve-x18 | FileCheck --check-prefix=RISCV32 %s
+
+; RUN: llc -verify-machineinstrs -o - %s -mtriple=riscv64-linux-gnu -mattr=+reserve-x18 | FileCheck --check-prefix=RISCV64 %s
+
+define void @f1() shadowcallstack {
+  ; CHECK: f1:
+  ; CHECK-NOT: x18
+  ; CHECK: ret
+  ret void
+}
+
+declare void @foo()
+
+define void @f2() shadowcallstack {
+  ; CHECK: f2:
+  ; CHECK-NOT: x18
+  ; CHECK: tail foo
+  tail call void @foo()
+  ret void
+}
+
+declare i32 @bar()
+
+define i32 @f3() shadowcallstack {
+  ; CHECK: f3:
+  ; RISCV32:		sw	ra, 0(s2)
+  ; RISCV32-NEXT:	addi	s2, s2, 4
+  ; RISCV64:		sw	ra, 0(s2)
+  ; RISCV64-NEXT:	addi	s2, s2, 8
+  ; CHECK:	addi	sp, sp, -16
+  ; CHECK:	sw	ra, 12(sp)
+  %res = call i32 @bar()
+  %res1 = add i32 %res, 1
+  ; CHECK:	lw	ra, 12(sp)
+  ; CHECK:	addi	sp, sp, 16
+  ; RISCV32:		addi	s2, s2, -4
+  ; RISCV32-NEXT:	lw	ra, 0(s2)
+  ; RISCV64:		addi	s2, s2, -8
+  ; RISCV64-NEXT:	lw	ra, 0(s2)
+  ; CHECK:	ret
+  ret i32 %res
+}
+
+define i32 @f4() shadowcallstack {
+  ; CHECK: f4:
+  ; RISCV32:		sw	ra, 0(s2)
+  ; RISCV32-NEXT:	addi	s2, s2, 4
+  ; RISCV64:		sw	ra, 0(s2)
+  ; RISCV64-NEXT:	addi	s2, s2, 8
+  ; CHECK:	addi	sp, sp, -16
+  ; CHECK:	sw	ra, 12(sp)
+  %res1 = call i32 @bar()
+  %res2 = call i32 @bar()
+  %res3 = call i32 @bar()
+  %res4 = call i32 @bar()
+  %res12 = add i32 %res1, %res2
+  %res34 = add i32 %res3, %res4
+  %res1234 = add i32 %res12, %res34
+  ; CHECK:	lw	ra, 12(sp)
+  ; CHECK: 	addi	sp, sp, 16
+  ; RISCV32: 		addi	s2, s2, -4
+  ; RISCV32-NEXT:	lw	ra, 0(s2)
+  ; RISCV64: 		addi	s2, s2, -8
+  ; RISCV64-NEXT:	lw	ra, 0(s2)
+  ; CHECK: 	ret
+  ret i32 %res1234
+}
+
+define i32 @f5() shadowcallstack nounwind {
+  ; CHECK: f5:
+  ; CHECK-NOT: .cfi_def_cfa_offset
+  ; RISCV32:		sw	ra, 0(s2)
+  ; RISCV32-NEXT:	addi	s2, s2, 4
+  ; RISCV64:		sw	ra, 0(s2)
+  ; RISCV64-NEXT:	addi	s2, s2, 8
+  ; CHECK:	addi	sp, sp, -16
+  ; CHECK:	sw	ra, 12(sp)
+  %res = call i32 @bar()
+  %res1 = add i32 %res, 1
+  ; CHECK:	lw	ra, 12(sp)
+  ; CHECK:	addi	sp, sp, 16
+  ; RISCV32:		addi	s2, s2, -4
+  ; RISCV32-NEXT:	lw	ra, 0(s2)
+  ; RISCV64:		addi	s2, s2, -8
+  ; RISCV64-NEXT:	lw	ra, 0(s2)
+  ; CHECK:	ret
+  ret i32 %res
+}
Index: llvm/lib/Target/RISCV/RISCVFrameLowering.cpp
===
--- llvm/lib/Target/RISCV/RISCVFrameLowering.cpp
+++ llvm/lib/Target/RISCV/RISCVFrameLowering.cpp
@@ -23,6 +23,78 @@
 
 using namespace llvm;
 
+// For now we use x18, a.k.a s2, as pointer to shadow call stack.
+// User should explicitly set -ffixed-x18 and not use x18 in their asm.
+static void emitSCSPrologue(MachineFunction &MF, MachineBasicBlock &MBB,
+MachineBasicBlock::iterator MI) {
+  if (!MF.getFunction().hasFnAttribute(Attribute::ShadowCallStack))
+return;
+
+  std::vector &CSI = MF.getFrameInfo().getCalleeSavedInfo();
+  if (std::none_of(CSI.begin(), CSI.end(),
+  [] (CalleeSavedInfo &CSR) { return CSR.getReg() == RISCV::X1; }))
+return;
+
+  const auto &STI = MF.getSubtarget();
+  // Emit an error message and bail out.
+  if (!STI.isRegisterReservedByUser(RISCV::X18)) {
+MF.getFunction().getContext().diagnose(DiagnosticInfoUnsupported{
+  MF.getFunction(), "x18 not reserved by user for Shadow Call Stack."});
+return;
+  }
+
+  DebugLoc DL = MI != MBB.end() ? MI->getDebugLoc() : DebugLoc();
+
+  const RISCVInstrInfo *TII = STI.getInstrInfo();
+  int64_t SlotSize = STI.getXLen() / 8;
+  // Store return address to shadow call stack
+  // sw   ra, 0(s2)
+  // addi s2, s2, 4
+  BuildMI(MBB, MI, DL, TII->get(RISCV::SW))
+  .addReg(RISCV::X1)
+  .addReg(RISCV::X18)
+  .addImm(0);
+  BuildMI(MBB, MI, DL, TII->get(RISCV::ADDI))
+  .addReg(RISCV::X18, RegState::Define)
+  .addReg(RI

[PATCH] D84414: [RISCV] Support Shadow Call Stack

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/test/CodeGen/shadowcallstack-attr.c:8
 
+// RUN: %clang_cc1 -triple riscv32-linux-gnu -emit-llvm -o - %s 
-fsanitize=shadow-call-stack | FileCheck -check-prefix=UNBLACKLISTED %s
+

Now might be a good opportunity to update this check prefix to a less loaded 
term.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84414



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

[PATCH] D84415: [analyzer][StdLibraryFunctionsChecker] Add POSIX pthread handling functions

[Gabor Marton via Phabricator via cfe-commits]

martong created this revision.
martong added reviewers: Szelethus, gamesh411, balazske.
Herald added subscribers: cfe-commits, ASDenysPetrov, steakhal, Charusso, 
dkrupp, donat.nagy, mikhail.ramalho, a.sidorin, rnkovacs, szepet, 
baloghadamsoftware, xazax.hun, whisperity.
Herald added a project: clang.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D84415

Files:
  clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
  clang/test/Analysis/std-c-library-functions-POSIX.c

Index: clang/test/Analysis/std-c-library-functions-POSIX.c
===
--- clang/test/Analysis/std-c-library-functions-POSIX.c
+++ clang/test/Analysis/std-c-library-functions-POSIX.c
@@ -95,6 +95,20 @@
 // CHECK: Loaded summary for: ssize_t send(int sockfd, const void *buf, size_t len, int flags)
 // CHECK: Loaded summary for: int socketpair(int domain, int type, int protocol, int sv[2])
 // CHECK: Loaded summary for: int getnameinfo(const struct sockaddr *restrict sa, socklen_t salen, char *restrict node, socklen_t nodelen, char *restrict service, socklen_t servicelen, int flags)
+// CHECK: Loaded summary for: int pthread_cond_signal(pthread_cond_t *cond)
+// CHECK: Loaded summary for: int pthread_cond_broadcast(pthread_cond_t *cond)
+// CHECK: Loaded summary for: int pthread_create(pthread_t *restrict thread, const pthread_attr_t *restrict attr, void *(*start_routine)(void *), void *restrict arg)
+// CHECK: Loaded summary for: int pthread_attr_destroy(pthread_attr_t *attr)
+// CHECK: Loaded summary for: int pthread_attr_init(pthread_attr_t *attr)
+// CHECK: Loaded summary for: int pthread_attr_getstacksize(const pthread_attr_t *restrict attr, size_t *restrict stacksize)
+// CHECK: Loaded summary for: int pthread_attr_getguardsize(const pthread_attr_t *restrict attr, size_t *restrict guardsize)
+// CHECK: Loaded summary for: int pthread_attr_setstacksize(pthread_attr_t *attr, size_t stacksize)
+// CHECK: Loaded summary for: int pthread_attr_setguardsize(pthread_attr_t *attr, size_t guardsize)
+// CHECK: Loaded summary for: int pthread_mutex_init(pthread_mutex_t *restrict mutex, const pthread_mutexattr_t *restrict attr)
+// CHECK: Loaded summary for: int pthread_mutex_destroy(pthread_mutex_t *mutex)
+// CHECK: Loaded summary for: int pthread_mutex_lock(pthread_mutex_t *mutex)
+// CHECK: Loaded summary for: int pthread_mutex_trylock(pthread_mutex_t *mutex)
+// CHECK: Loaded summary for: int pthread_mutex_unlock(pthread_mutex_t *mutex)
 
 long a64l(const char *str64);
 char *l64a(long value);
@@ -227,6 +241,26 @@
 int socketpair(int domain, int type, int protocol, int sv[2]);
 int getnameinfo(const struct sockaddr *restrict sa, socklen_t salen, char *restrict node, socklen_t nodelen, char *restrict service, socklen_t servicelen, int flags);
 
+typedef union { int x; } pthread_cond_t;
+int pthread_cond_signal(pthread_cond_t *cond);
+int pthread_cond_broadcast(pthread_cond_t *cond);
+typedef union { int x; } pthread_attr_t;
+typedef unsigned long int pthread_t;
+int pthread_create(pthread_t *restrict thread, const pthread_attr_t *restrict attr, void *(*start_routine)(void *), void *restrict arg);
+int pthread_attr_destroy(pthread_attr_t *attr);
+int pthread_attr_init(pthread_attr_t *attr);
+int pthread_attr_getstacksize(const pthread_attr_t *restrict attr, size_t *restrict stacksize);
+int pthread_attr_getguardsize(const pthread_attr_t *restrict attr, size_t *restrict guardsize);
+int pthread_attr_setstacksize(pthread_attr_t *attr, size_t stacksize);
+int pthread_attr_setguardsize(pthread_attr_t *attr, size_t guardsize);
+typedef union { int x; } pthread_mutex_t;
+typedef union { int x; } pthread_mutexattr_t;
+int pthread_mutex_init(pthread_mutex_t *restrict mutex, const pthread_mutexattr_t *restrict attr);
+int pthread_mutex_destroy(pthread_mutex_t *mutex);
+int pthread_mutex_lock(pthread_mutex_t *mutex);
+int pthread_mutex_trylock(pthread_mutex_t *mutex);
+int pthread_mutex_unlock(pthread_mutex_t *mutex);
+
 // Must have at least one call expression to initialize the summary map.
 int bar(void);
 void foo() {
Index: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
@@ -812,6 +812,8 @@
   const QualType ConstWchar_tPtrTy =
   ACtx.getPointerType(ACtx.WCharTy.withConst()); // const wchar_t *
   const QualType ConstVoidPtrRestrictTy = getRestrictTy(ConstVoidPtrTy);
+  const QualType SizePtrTy = ACtx.getPointerType(SizeTy);
+  const QualType SizePtrRestrictTy = getRestrictTy(SizePtrTy);
 
   const RangeInt IntMax = BVF.getMaxValue(IntTy).getLimitedValue();
   const RangeInt UnsignedIntMax =
@@ -883,6 +885,11 @@
   for (const Summary &S : Summaries)
 operator()(Name, S);
 }
+// Add the same summary for different names.
+ 

[PATCH] D84414: [RISCV] Support Shadow Call Stack

[Jessica Clarke via Phabricator via cfe-commits]

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



Comment at: llvm/test/CodeGen/RISCV/shadowcallstack.ll:1-3
+; RUN: llc -verify-machineinstrs -o - %s -mtriple=riscv32-linux-gnu 
-mattr=+reserve-x18 | FileCheck --check-prefix=RISCV32 %s
+
+; RUN: llc -verify-machineinstrs -o - %s -mtriple=riscv64-linux-gnu 
-mattr=+reserve-x18 | FileCheck --check-prefix=RISCV64 %s

Please style these in the same way as the other RISC-V CodeGen tests, in terms 
of argument order, redirecting stdin rather than using `-o - %s`, and using 
riscv64 rather than riscv64-linux-gnu (unless needed?). Also use 
update_llc_test_checks.py rather than hand-writing this. And we generally use 
RV32I and RV64I (or other appropriate arch strings) instead of RISCV32 and 
RISCV64 prefixes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84414



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

[PATCH] D84306: [clang-format][NFC] Be more careful about the layout of FormatToken.

[Bruno Ricci via Phabricator via cfe-commits]

riccibruno marked 3 inline comments as done.
riccibruno added inline comments.



Comment at: clang/lib/Format/ContinuationIndenter.cpp:652
+  (Current.isNot(TT_LineComment) ||
+   Previous.getBlockKind() == BK_BracedInit)) {
 State.Stack.back().Indent = State.Column + Spaces;

MyDeveloperDay wrote:
> I think this is better in that its now easier perhaps to see when the block 
> kind gets checked:
> 
> I wonder if it would read even better as if we added `is(BraceBlockKind)`   
> `isNot(BraceBlockKind)` 
> 
> e.g.
> 
> `Previous.is(BK_BraceInit)`
Yes I think I agree with you; adding `is` and `isNot` would be an improvement. 
I will still leave `getBlockKind` if someone wants to switch on it.



Comment at: clang/lib/Format/FormatToken.h:151
+BlockKind(BK_Unknown), Type(TT_Unknown), Decision(FD_Unformatted),
+PackingKind(PPK_Inconclusive) {}
 

MyDeveloperDay wrote:
> I much prefer putting the initialization here, I think it makes it MUCH 
> clearer
It is necessary anyway since a bit-field cannot have a default member 
initializer pre-C++20.



Comment at: clang/lib/Format/FormatToken.h:177
   /// Indicates that this is the first token of the file.
-  bool IsFirst = false;
+  unsigned IsFirst : 1;
 

MyDeveloperDay wrote:
> educate me, why
> 
> ```
> unsigned IsFirst : 1;
> ```
> 
> here and not
> 
> ```
> bool IsFirst : 1;
> ```
> 
> is that equivalent? (I'm literally not sure myself), I wrote a little test 
> just to remind myself how this stuff works.
> 
> ```
> #include 
> 
> class Foo
> {
> public:
> Foo()
>   : A(true)
>   , B(false)
>   , C(true)
> {
> }
> bool A : 1;
> bool B : 1;
> bool C : 1;
> };
> 
> class Bar
> {
> public:
> Bar()
>   : A(true)
>   , B(false)
>   , C(true)
> {
> }
> unsigned A : 1;
> unsigned B : 1;
> unsigned C : 1;
> };
> 
> class Fuz
> {
> public:
> Fuz()
>   : A(true)
>   , B(false)
>   , C(true)
> {
> }
> bool A;
> bool B;
> bool C;
> };
> 
> class Baz
> {
> public:
> Baz()
>   : A(true)
>   , B(false)
>   , C(true)
> {
> }
> unsigned A;
> unsigned B;
> unsigned C;
> };
> 
> int
> main(int argc, char *argv[])
> {
> std::cerr << "Foo " << sizeof(Foo) << "\n";
> std::cerr << "Bar " < std::cerr << "Fuz " < std::cerr << "Baz " < 
> return 0;
> }
> ```
> 
> When run gives the following:
> 
> ```
> Foo 1
> Bar 4
> Fuz 3
> Baz 12
> ```
> 
> So I guess my question is could there be more space savings if using bool 
> IsFirst:1  (and the others), I'd also think that would help clarity a little 
> (or did I misunderstand?)
> 
> 
It has to do with the ABI since as per [class.bit]p1:

`[...] Allocation of bit-fields within a class object is 
implementation-defined. Alignment of bit fields is implementation-defined. 
Bit-fields are packed into some addressable allocation unit. [Note: Bit-fields 
straddle allocation units on some machines and not on others. Bit-fields are 
assigned right-to-left on some machines, left-to-right on others. — end note ]`

Now the two relevant ABIs are the Itanium ABI 
(https://github.com/itanium-cxx-abi/cxx-abi/blob/master/abi-layout.html for the 
details) and the MS ABI (say on x86-64). Happily both are supported by clang so 
we can use `-fdump-record-layouts` and compare them.

Consider `S0` in https://godbolt.org/z/orYv5j (itanium on the left/MS on the 
right):

Both ABIs will not let a bit-field cross a boundary. Therefore `c0` and `c1` 
will use 10 bits. If the type had been `short` instead of `char` then only 9 
bits would have been used. The size of `S0` would still have been 2 in both 
cases.

Consider now `S1`. The MS ABI, unlike the Itanium ABI, will not put bit-fields 
together if their types have a different size. Therefore `sizeof(S1)` is 2 
under the Itanium ABI and `4` under the MS ABI.

Using an `unsigned` systematically avoids having a boundary every 8 bits, and 
avoids the issue with the MS ABI.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84306



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

[PATCH] D84414: [RISCV] Support Shadow Call Stack

[Jessica Clarke via Phabricator via cfe-commits]

jrtc27 added a comment.

Is there a reason for choosing X18? On AArch64 that's either a temporary or 
saved register depending on ABI, but determined to be the "platform register". 
Picking X18 on RISC-V because that's the same index as AArch64 seems a little 
arbitrary, but maybe it happens to make sense.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84414



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

[PATCH] D79279: Add overloaded versions of builtin mem* functions

[JF Bastien via Phabricator via cfe-commits]

jfb added a comment.

In D79279#2168649 , @rjmccall wrote:

> In D79279#2168533 , @jfb wrote:
>
> > In D79279#2168479 , @rjmccall 
> > wrote:
> >
> > > Is there a need for an atomic memcpy at all?  Why is it useful to allow 
> > > this operation to take on "atomic" semantics — which aren't actually 
> > > atomic because the loads and stores to elements are torn — with hardcoded 
> > > memory ordering and somewhat arbitrary rules about what the atomic size 
> > > is?
> >
> >
> > Hans lays out a rationale for usefulness in his paper, but what I've 
> > implemented is more useful: it's *unordered* so you can fence as you desire 
> > around it, yet it guarantees a minimum memory access size based on the 
> > pointer parameters. For example, copying an atomic `int` will be 4 byte 
> > operations which are single-copy-atomic, but the accesses from one `int` to 
> > the next aren't performed in any guaranteed order (or observable in any 
> > guaranteed order either). I talked about this with him a while ago but IIRC 
> > he wasn't sure about implementation among other things, so when you asked 
> > me to widen my original `volatile`-only `memcpy` to also do other 
> > qualifiers, I realized that it was a neat way to do atomic as well as other 
> > qualifiers. I've talked to a few SG1 folks about this, and I believe (for 
> > other reasons too) it's where the design will end up for Hans' paper.
>
>
> I can see the usefulness of this operation, but it seems like a odd semantic 
> mismatch for what is basically just a memcpy where one of the pointers 
> happens to have `_Atomic` type, like you're shoe-horning it into this builtin 
> just to avoid declaring a different one.


I'm following the discussion we had here regarding overloading 
:

>> There are other qualifiers that can meaningfully contribute to the operation 
>> here besides volatile, such as restrict and (more importantly) address 
>> spaces. And again, for the copy operations these might differ between the 
>> two pointer types.
>> 
>> In both cases, I’d say that the logical design is to allow the pointers to 
>> be to arbitrarily-qualified types. We can then propagate that information 
>> from the builtin into the LLVM intrinsic call as best as we’re allowed. So I 
>> think you should make builtins called something like 
>> __builtin_overloaded_memcpy (name to be decided) and just have their 
>> semantics be type-directed.
> 
> Ah yes, I’d like to hear what others think of this. I hadn’t thought about it 
> before you brought it up, and it sounds like a good idea.

As you noted earlier, for `memcpy` you probably want to express differences in 
destination and source qualification, even if today IR can't express e.g. 
`volatile` source and non-`volatile` destination. You were talking about 
`volatile`, but this applies to the entire combination of dst+src qualified 
with zero-to-five `volatile`, `_Atomic`, `__unaligned`, `restrict`, and address 
space. Pulling the entire combination space out into different functions would 
create way too many functions. Right now the implementation has a few 
limitations: it treats both dst and src as `volatile` if either are, it can't 
do `_Atomic` with `volatile` so we diagnose, and it ignores `restrict`.  
Otherwise it supports all combinations.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79279



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

[PATCH] D83281: [OpenMP] Allow traits for the OpenMP context selector `isa`

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert added a comment.

ping :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83281



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

[PATCH] D80858: [CUDA][HIP] Support accessing static device variable in host code

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added a comment.

Another reason is that we need to support it in rdc mode, where different TU 
can have static var with the same name.


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

https://reviews.llvm.org/D80858



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

[PATCH] D82582: [SVE] Remove calls to VectorType::getNumElements from clang

[Christopher Tetreault via Phabricator via cfe-commits]

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



Comment at: clang/lib/CodeGen/CGBuiltin.cpp:5612-5613
 
   llvm::VectorType *VTy = GetNeonType(this, Type, HasLegalHalfType, false,
   AllowBFloatArgsAndRet);
   llvm::Type *Ty = VTy;

c-rhodes wrote:
> Is there a reason you can't do a single cast here?
GetNeonType should probably just return a FixedVectorType.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82582



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

[PATCH] D79279: Add overloaded versions of builtin mem* functions

[JF Bastien via Phabricator via cfe-commits]

jfb updated this revision to Diff 280135.
jfb added a comment.

Improve documentation


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79279

Files:
  clang/docs/LanguageExtensions.rst


Index: clang/docs/LanguageExtensions.rst
===
--- clang/docs/LanguageExtensions.rst
+++ clang/docs/LanguageExtensions.rst
@@ -2454,6 +2454,14 @@
 * Implement an atomic memory with atomic operations of a particular size,
   similar to that presented in C++ proposal [p1478](https://wg21.link/p1478).
 
+When using the `_Atomic` qualifier, the memory will be accessed with a sequence
+of operations of size equal to or a multiple of the pointer's element size. The
+order of operations is unspecified, and has unordered atomic semantics. This
+means that reads and writes do not tear at the individual element level, and
+they each occur exactly once, but the order in which they occur can only be
+guaranteed using appropriate fences. Atomic memory operations must be to memory
+locations which are aligned to no less than the element size.
+
 Atomic Min/Max builtins with memory ordering
 
 


Index: clang/docs/LanguageExtensions.rst
===
--- clang/docs/LanguageExtensions.rst
+++ clang/docs/LanguageExtensions.rst
@@ -2454,6 +2454,14 @@
 * Implement an atomic memory with atomic operations of a particular size,
   similar to that presented in C++ proposal [p1478](https://wg21.link/p1478).
 
+When using the `_Atomic` qualifier, the memory will be accessed with a sequence
+of operations of size equal to or a multiple of the pointer's element size. The
+order of operations is unspecified, and has unordered atomic semantics. This
+means that reads and writes do not tear at the individual element level, and
+they each occur exactly once, but the order in which they occur can only be
+guaranteed using appropriate fences. Atomic memory operations must be to memory
+locations which are aligned to no less than the element size.
+
 Atomic Min/Max builtins with memory ordering
 
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D79279: Add overloaded versions of builtin mem* functions

[JF Bastien via Phabricator via cfe-commits]

jfb updated this revision to Diff 280136.
jfb added a comment.

Re-update


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79279

Files:
  clang/docs/LanguageExtensions.rst
  clang/include/clang/Basic/Builtins.def
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/Sema.h
  clang/lib/CodeGen/CGBuilder.h
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/Sema/SemaChecking.cpp
  clang/test/CodeGen/builtin-overloaded-memfns.c
  clang/test/CodeGenObjC/builtin-memfns.m
  clang/test/Sema/builtin-overloaded-memfns.cpp
  clang/test/SemaOpenCL/invalid-pipe-builtin-cl2.0.cl
  clang/test/SemaOpenCL/to_addr_builtin.cl

Index: clang/test/SemaOpenCL/to_addr_builtin.cl
===
--- clang/test/SemaOpenCL/to_addr_builtin.cl
+++ clang/test/SemaOpenCL/to_addr_builtin.cl
@@ -15,7 +15,7 @@
   // expected-error@-2{{implicit declaration of function 'to_global' is invalid in OpenCL}}
   // expected-warning@-3{{incompatible integer to pointer conversion assigning to '__global int *__private' from 'int'}}
 #else
-  // expected-error@-5{{invalid number of arguments to function: 'to_global'}}
+  // expected-error@-5{{too many arguments to function call, expected 1, have 2}}
 #endif
 
   int x;
Index: clang/test/SemaOpenCL/invalid-pipe-builtin-cl2.0.cl
===
--- clang/test/SemaOpenCL/invalid-pipe-builtin-cl2.0.cl
+++ clang/test/SemaOpenCL/invalid-pipe-builtin-cl2.0.cl
@@ -10,7 +10,7 @@
   read_pipe(p, &tmp);
   read_pipe(p, ptr);
   read_pipe(tmp, p);// expected-error {{first argument to 'read_pipe' must be a pipe type}}
-  read_pipe(p);   // expected-error {{invalid number of arguments to function: 'read_pipe'}}
+  read_pipe(p); // expected-error {{invalid number of arguments to function: 'read_pipe'}}
   read_pipe(p, rid, tmp, ptr);
   read_pipe(p, tmp, tmp, ptr);   // expected-error {{invalid argument type to function 'read_pipe' (expecting 'reserve_id_t' having '__private int')}}
   read_pipe(p, rid, rid, ptr);   // expected-error {{invalid argument type to function 'read_pipe' (expecting 'unsigned int' having '__private reserve_id_t')}}
@@ -39,7 +39,7 @@
   write_pipe(p, &tmp);
   write_pipe(p, ptr);
   write_pipe(tmp, p);// expected-error {{first argument to 'write_pipe' must be a pipe type}}
-  write_pipe(p);   // expected-error {{invalid number of arguments to function: 'write_pipe'}}
+  write_pipe(p); // expected-error {{invalid number of arguments to function: 'write_pipe'}}
   write_pipe(p, rid, tmp, ptr);
   write_pipe(p, tmp, tmp, ptr);   // expected-error {{invalid argument type to function 'write_pipe' (expecting 'reserve_id_t' having '__private int')}}
   write_pipe(p, rid, rid, ptr);   // expected-error {{invalid argument type to function 'write_pipe' (expecting 'unsigned int' having '__private reserve_id_t')}}
Index: clang/test/Sema/builtin-overloaded-memfns.cpp
===
--- /dev/null
+++ clang/test/Sema/builtin-overloaded-memfns.cpp
@@ -0,0 +1,222 @@
+// RUN: %clang_cc1 %s -verify -fsyntax-only -triple=arm64-unknown-unknown -fms-extensions -DCPY=1
+// RUN: %clang_cc1 %s -verify -fsyntax-only -triple=arm64-unknown-unknown -fms-extensions -DCPY=0
+
+// Test memcpy and memmove with the same code, since they're basically the same constraints.
+#if CPY
+#define MEM(...) __builtin_memcpy_overloaded(__VA_ARGS__)
+#else
+#define MEM(...) __builtin_memmove_overloaded(__VA_ARGS__)
+#endif
+
+#define NULL (void *)0
+#define nullptr __nullptr
+using size_t = __SIZE_TYPE__;
+using sizeless_t = __SVInt8_t;
+using float4 = float __attribute__((ext_vector_type(4)));
+struct Intish {
+  int i;
+};
+struct NotLockFree {
+  char buf[512];
+};
+struct TrivialCpy {
+  char buf[8];
+  TrivialCpy();
+  TrivialCpy(const TrivialCpy &) = default;
+};
+struct NotTrivialCpy {
+  char buf[8];
+  NotTrivialCpy();
+  NotTrivialCpy(const NotTrivialCpy &);
+};
+
+void arg_count() {
+  MEM();   // expected-error {{too few arguments to function call, expected 3, have 0}}
+  MEM(0);  // expected-error {{too few arguments to function call, expected 3, have 1}}
+  MEM(0, 0);   // expected-error {{too few arguments to function call, expected 3, have 2}}
+  MEM(0, 0, 0, 0); // expected-error {{too many arguments to function call, expected 3, have 4}}
+  __builtin_memset_overloaded();   // expected-error {{too few arguments to function call, expected 3, have 0}}
+  __builtin_memset_overloaded(0);  // expected-error {{too few arguments to function call, expected 3, have 1}}
+  __builtin_memset_overloaded(0, 0);   // expected-error {{too few arguments to function call, expected 3, have 2}}
+

[PATCH] D83281: [OpenMP] Allow traits for the OpenMP context selector `isa`

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.



Comment at: clang/include/clang/AST/OpenMPClause.h:7599
+  /// (which accept anything) and (later) extensions.
+  StringRef RawString{};
 };

No need for the default initializer here



Comment at: clang/include/clang/AST/OpenMPClause.h:7658
+/// Clang specific specialization of the OMPContext to lookup target features.
+struct TargetOMPContext : public llvm::omp::OMPContext {
+

`final` and virtual default destructor?



Comment at: clang/include/clang/AST/OpenMPClause.h:7665
+  /// See llvm::omp::OMPContext::matchesISATrait
+  bool matchesISATrait(StringRef RawString) const;
+

`override`?



Comment at: clang/lib/Parse/ParseOpenMP.cpp:1826
+
+std::function DiagUnknownTrait = [&](StringRef ISATrait) {
+  // TODO Track the selector locations in a way that is accessible here to

Seems to me, `SourceLocation` is usually captured by value.



Comment at: clang/lib/Sema/SemaOpenMP.cpp:5894
   ASTContext &Context = getASTContext();
-  OMPContext OMPCtx(getLangOpts().OpenMPIsDevice,
-Context.getTargetInfo().getTriple());
+  std::function DiagUnknownTrait = [&](StringRef ISATrait) {
+// TODO Track the selector locations in a way that is accessible here to

`CE` can be captured by value



Comment at: llvm/lib/Frontend/OpenMP/OMPContext.cpp:205
+if (Property == TraitProperty::device_isa___ANY)
+  IsActiveTrait = std::all_of(
+  VMI.ISATraits.begin(), VMI.ISATraits.end(),

`llvm::all_of`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83281



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

[PATCH] D84364: [CUDA][HIP] Defer all diagnostics for host device functions

[Yaxun Liu via Phabricator via cfe-commits]

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

update the lit test for SFINAE. make sure substitution failure does not incur 
error msg if there is valid substitution.

Since template is not allowed in local class, there is no need for test SFINAE 
inside host device function.


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

https://reviews.llvm.org/D84364

Files:
  clang/include/clang/Basic/LangOptions.def
  clang/include/clang/Driver/Options.td
  clang/include/clang/Sema/Sema.h
  clang/lib/Driver/ToolChains/Cuda.cpp
  clang/lib/Driver/ToolChains/HIP.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/Sema/AnalysisBasedWarnings.cpp
  clang/lib/Sema/Sema.cpp
  clang/lib/Sema/SemaAttr.cpp
  clang/lib/Sema/SemaCUDA.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaExprObjC.cpp
  clang/lib/Sema/SemaOpenMP.cpp
  clang/lib/Sema/SemaSYCL.cpp
  clang/lib/Sema/SemaStmt.cpp
  clang/lib/Sema/SemaStmtAsm.cpp
  clang/lib/Sema/SemaTemplateInstantiate.cpp
  clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
  clang/lib/Sema/SemaTemplateVariadic.cpp
  clang/lib/Sema/SemaType.cpp
  clang/test/SemaCUDA/deferred-all.cu

Index: clang/test/SemaCUDA/deferred-all.cu
===
--- /dev/null
+++ clang/test/SemaCUDA/deferred-all.cu
@@ -0,0 +1,74 @@
+// RUN: %clang_cc1 -fcuda-is-device -fsyntax-only -verify=dev,com %s \
+// RUN:   -std=c++11 -fgpu-defer-diag
+// RUN: %clang_cc1 -fsyntax-only -verify=host,com %s \
+// RUN:   -std=c++11 -fgpu-defer-diag
+
+#include "Inputs/cuda.h"
+
+__device__ void callee(int);
+__host__ void callee(float); // host-note 2{{candidate function}}
+__host__ void callee(double); // host-note 2{{candidate function}}
+
+// Check no diagnostics for this function since it is never
+// called.
+
+inline __host__ __device__ void hdf_not_called() {
+  callee(1);
+  bad_line
+}
+
+// When emitted on device, there is syntax error.
+// When emitted on host, there is ambiguity and syntax error.
+  
+inline __host__ __device__ void hdf_called() {
+  callee(1); // host-error {{call to 'callee' is ambiguous}}
+  bad_line // com-error {{use of undeclared identifier 'bad_line'}}
+}
+
+// This is similar to the above but is always emitted on
+// both sides.
+
+__host__ __device__ void hdf_always_emitted() {
+  callee(1); // host-error {{call to 'callee' is ambiguous}}
+  bad_line // com-error {{use of undeclared identifier 'bad_line'}}
+}
+
+void hf() {
+ hdf_called(); // host-note {{called by 'hf'}}
+}
+ 
+__device__ void df() {
+ hdf_called(); // dev-note {{called by 'df'}}
+}
+
+struct A { int x; typedef int type; };
+struct B { int x; };
+
+// This function is invalid for A and B by SFINAE.
+// This fails to substitue for A but no diagnostic
+// should be emitted.
+template
+__host__ __device__ void sfinae(T t) { // com-note {{candidate template ignored: substitution failure [with T = B]}}
+  t.x = 1;
+}
+
+// This function is defined for A only by SFINAE.
+// Calling it with A should succeed, with B should fail.
+// The error should not be deferred since it happens in
+// file scope.
+
+template
+__host__ __device__ void sfinae(T t) { // com-note {{candidate template ignored: substitution failure [with T = B]}}
+  t.x = 1;
+}
+
+void test_sfinae() {
+  sfinae(A());
+  sfinae(B()); // com-error{{no matching function for call to 'sfinae'}}
+}
+
+// If a syntax error causes a function not declared, it cannot
+// be deferred.
+
+inline __host__ __device__ void bad_func() { // com-note {{to match this '{'}}
+// com-error {{expected '}'}}
Index: clang/lib/Sema/SemaType.cpp
===
--- clang/lib/Sema/SemaType.cpp
+++ clang/lib/Sema/SemaType.cpp
@@ -4087,7 +4087,8 @@
 
 /// Creates a fix-it to insert a C-style nullability keyword at \p pointerLoc,
 /// taking into account whitespace before and after.
-static void fixItNullability(Sema &S, DiagnosticBuilder &Diag,
+template 
+static void fixItNullability(Sema &S, DiagBuilderT &Diag,
  SourceLocation PointerLoc,
  NullabilityKind Nullability) {
   assert(PointerLoc.isValid());
Index: clang/lib/Sema/SemaTemplateVariadic.cpp
===
--- clang/lib/Sema/SemaTemplateVariadic.cpp
+++ clang/lib/Sema/SemaTemplateVariadic.cpp
@@ -368,8 +368,8 @@
   Locations.push_back(Unexpanded[I].second);
   }
 
-  DiagnosticBuilder DB = Diag(Loc, diag::err_unexpanded_parameter_pack)
- << (int)UPPC << (int)Names.size();
+  auto DB = Diag(Loc, diag::err_unexpanded_parameter_pack)
+<< (int)UPPC << (int)Names.size();
   for (size_t I = 0, E = std::min(Names.size(), (size_t)2); I != E; ++I)
 DB << Names[I];
 
Index: clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
===
--- clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
+

[PATCH] D83281: [OpenMP] Allow traits for the OpenMP context selector `isa`

[Johannes Doerfert via Phabricator via cfe-commits]

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

Addressed @ABataev comments, thx!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83281

Files:
  clang/include/clang/AST/OpenMPClause.h
  clang/include/clang/Basic/DiagnosticParseKinds.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/AST/OpenMPClause.cpp
  clang/lib/Parse/ParseOpenMP.cpp
  clang/lib/Sema/SemaOpenMP.cpp
  clang/test/OpenMP/declare_variant_device_isa_codegen_1.c
  clang/test/OpenMP/declare_variant_messages.c
  llvm/include/llvm/Frontend/OpenMP/OMPContext.h
  llvm/include/llvm/Frontend/OpenMP/OMPKinds.def
  llvm/lib/Frontend/OpenMP/OMPContext.cpp
  llvm/unittests/Frontend/OpenMPContextTest.cpp

Index: llvm/unittests/Frontend/OpenMPContextTest.cpp
===
--- llvm/unittests/Frontend/OpenMPContextTest.cpp
+++ llvm/unittests/Frontend/OpenMPContextTest.cpp
@@ -38,11 +38,13 @@
 #define OMP_TRAIT_PROPERTY(Enum, TraitSetEnum, TraitSelectorEnum, Str) \
   EXPECT_EQ(TraitProperty::Enum,   \
 getOpenMPContextTraitPropertyKind( \
-TraitSet::TraitSetEnum,\
-getOpenMPContextTraitPropertyName(TraitProperty::Enum)));  \
+TraitSet::TraitSetEnum, TraitSelector::TraitSelectorEnum,  \
+getOpenMPContextTraitPropertyName(TraitProperty::Enum, Str))); \
   EXPECT_EQ(Str, getOpenMPContextTraitPropertyName(\
- getOpenMPContextTraitPropertyKind(TraitSet::TraitSetEnum, \
-   Str))); \
+ getOpenMPContextTraitPropertyKind(\
+ TraitSet::TraitSetEnum,   \
+ TraitSelector::TraitSelectorEnum, Str),   \
+ Str));\
   EXPECT_EQ(TraitSet::TraitSetEnum,\
 getOpenMPContextTraitSetForProperty(TraitProperty::Enum)); \
   EXPECT_EQ(TraitSelector::TraitSelectorEnum,  \
Index: llvm/lib/Frontend/OpenMP/OMPContext.cpp
===
--- llvm/lib/Frontend/OpenMP/OMPContext.cpp
+++ llvm/lib/Frontend/OpenMP/OMPContext.cpp
@@ -175,11 +175,11 @@
 LLVM_DEBUG({
   if (MK == MK_ALL)
 dbgs() << "[" << DEBUG_TYPE << "] Property "
-   << getOpenMPContextTraitPropertyName(Property)
+   << getOpenMPContextTraitPropertyName(Property, "")
<< " was not in the OpenMP context but match kind is all.\n";
   if (MK == MK_NONE)
 dbgs() << "[" << DEBUG_TYPE << "] Property "
-   << getOpenMPContextTraitPropertyName(Property)
+   << getOpenMPContextTraitPropertyName(Property, "")
<< " was in the OpenMP context but match kind is none.\n";
 });
 return false;
@@ -198,6 +198,14 @@
   continue;
 
 bool IsActiveTrait = Ctx.ActiveTraits.test(unsigned(Property));
+
+// We overwrite the isa trait as it is actually up to the OMPContext hook to
+// check the raw string(s).
+if (Property == TraitProperty::device_isa___ANY)
+  IsActiveTrait = llvm::all_of(VMI.ISATraits, [&](StringRef RawString) {
+return Ctx.matchesISATrait(RawString);
+  });
+
 Optional Result = HandleTrait(Property, IsActiveTrait);
 if (Result.hasValue())
   return Result.getValue();
@@ -225,7 +233,7 @@
 
   if (!FoundInOrder) {
 LLVM_DEBUG(dbgs() << "[" << DEBUG_TYPE << "] Construct property "
-  << getOpenMPContextTraitPropertyName(Property)
+  << getOpenMPContextTraitPropertyName(Property, "")
   << " was not nested properly.\n");
 return false;
   }
@@ -425,8 +433,12 @@
   llvm_unreachable("Unknown trait selector!");
 }
 
-TraitProperty llvm::omp::getOpenMPContextTraitPropertyKind(TraitSet Set,
-   StringRef S) {
+TraitProperty llvm::omp::getOpenMPContextTraitPropertyKind(
+TraitSet Set, TraitSelector Selector, StringRef S) {
+  // Special handling for `device={isa(...)}` as we accept anything here. It is
+  // up to the target to decide if the feature is available.
+  if (Set == TraitSet::device && Selector == TraitSelector::device_isa)
+return TraitProperty::device_isa___ANY;
 #define OMP_TRAIT_PROPERTY(Enum, TraitSetEnum, TraitSelectorEnum, Str) \
   if (Set == TraitSet::TraitSetEnum && Str == S)   \
   

[PATCH] D83961: [Analyzer] Fix bug report source locations in minimal output.

[Balázs Kéri via Phabricator via cfe-commits]

balazske updated this revision to Diff 280149.
balazske added a comment.

Change column in malloc-plist test because code was reformatted.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83961

Files:
  clang/lib/Analysis/PathDiagnostic.cpp
  clang/lib/StaticAnalyzer/Core/BugReporter.cpp
  clang/test/Analysis/CGColorSpace.c
  clang/test/Analysis/Inputs/expected-plists/malloc-plist.c.plist
  clang/test/Analysis/NSString.m
  clang/test/Analysis/malloc-plist.c

Index: clang/test/Analysis/malloc-plist.c
===
--- clang/test/Analysis/malloc-plist.c
+++ clang/test/Analysis/malloc-plist.c
@@ -135,8 +135,8 @@
 static void function_with_leak3(int y) {
 char *x = (char*)malloc(12);
 if (y)
-y++;
-}//expected-warning{{Potential leak}}
+  y++; //expected-warning{{Potential leak}}
+}
 void use_function_with_leak3(int y) {
 function_with_leak3(y);
 }
Index: clang/test/Analysis/NSString.m
===
--- clang/test/Analysis/NSString.m
+++ clang/test/Analysis/NSString.m
@@ -125,13 +125,13 @@
 
 NSString* f7(NSString* s1, NSString* s2, NSString* s3) {
 
-  NSString* s4 = (NSString*)
-CFStringCreateWithFormat(kCFAllocatorDefault, 0,  // expected-warning{{leak}}
- (CFStringRef) __builtin___CFStringMakeConstantString("%@ %@ (%@)"), 
- s1, s2, s3);
+  NSString *s4 = (NSString *)
+  CFStringCreateWithFormat(kCFAllocatorDefault, 0,
+   (CFStringRef)__builtin___CFStringMakeConstantString("%@ %@ (%@)"),
+   s1, s2, s3);
 
   CFRetain(s4);
-  return s4;
+  return s4; // expected-warning{{leak}}
 }
 
 NSMutableArray* f8() {
@@ -202,15 +202,15 @@
 }
 - (void)m1
 {
- NSString *s = [[NSString alloc] init]; // expected-warning{{leak}}
- [s retain];
- [s autorelease];
-}
+  NSString *s = [[NSString alloc] init];
+  [s retain];
+  [s autorelease];
+} // expected-warning{{leak}}
 - (void)m2
 {
- NSString *s = [[[NSString alloc] init] autorelease]; // expected-warning{{leak}}
- [s retain];
-}
+  NSString *s = [[[NSString alloc] init] autorelease];
+  [s retain];
+} // expected-warning{{leak}}
 - (void)m3
 {
  NSString *s = [[[NSString alloc] init] autorelease];
@@ -219,9 +219,9 @@
 }
 - (void)m4
 {
- NSString *s = [[NSString alloc] init]; // expected-warning{{leak}}
- [s retain];
-}
+  NSString *s = [[NSString alloc] init];
+  [s retain];
+} // expected-warning{{leak}}
 - (void)m5
 {
  NSString *s = [[NSString alloc] init];
@@ -360,9 +360,9 @@
 }
 
 void test_objc_atomicCompareAndSwap_parameter_no_direct_release(NSString **old) {
-  NSString *s = [[NSString alloc] init]; // expected-warning{{leak}}
+  NSString *s = [[NSString alloc] init];
   if (!objc_atomicCompareAndSwapPtr(0, s, old))
-return;
+return; // expected-warning{{leak}}
   else
 [*old release];
 }
@@ -382,8 +382,8 @@
 @end
 
 void test_isTrackedObjectType(void) {
-  NSString *str = [TestIsTracked newString]; // expected-warning{{Potential leak}}
-}
+  NSString *str = [TestIsTracked newString];
+} // expected-warning{{Potential leak}}
 
 // Test isTrackedCFObjectType().
 @interface TestIsCFTracked
@@ -402,9 +402,9 @@
 // Test @synchronized
 void test_synchronized(id x) {
   @synchronized(x) {
-NSString *string = [[NSString stringWithFormat:@"%ld", (long) 100] retain]; // expected-warning {{leak}}
+NSString *string = [[NSString stringWithFormat:@"%ld", (long)100] retain];
   }
-}
+} // expected-warning {{leak}}
 @end
 
 void testOSCompareAndSwapXXBarrier_parameter(NSString **old) {
Index: clang/test/Analysis/Inputs/expected-plists/malloc-plist.c.plist
===
--- clang/test/Analysis/Inputs/expected-plists/malloc-plist.c.plist
+++ clang/test/Analysis/Inputs/expected-plists/malloc-plist.c.plist
@@ -3764,12 +3764,12 @@
  
   
line138
-   col9
+   col7
file0
   
   
line138
-   col9
+   col7
file0
   
  
@@ -3781,7 +3781,7 @@
  location
  
   line138
-  col9
+  col7
   file0
  
  depth1
@@ -3803,7 +3803,7 @@
   location
   
line138
-   col9
+   col7
file0
   
   ExecutedLines
Index: clang/test/Analysis/CGColorSpace.c
===
--- clang/test/Analysis/CGColorSpace.c
+++ clang/test/Analysis/CGColorSpace.c
@@ -6,9 +6,9 @@
 extern void CGColorSpaceRelease(CGColorSpaceRef space);
 
 void f() {
-  CGColorSpaceRef X = CGColorSpaceCreateDeviceRGB(); // expected-warning{{leak}}
+  CGColorSpaceRef X = CGColorSpaceCreateDeviceRGB();
   CGColorSpaceRetain(X);
-}
+} // expected-warning{{leak}}
 
 void fb() {
   CGColorSpaceRef X = CGColorSpaceCreateDev

[PATCH] D83281: [OpenMP] Allow traits for the OpenMP context selector `isa`

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.



Comment at: clang/include/clang/AST/OpenMPClause.h:7663
+   const FunctionDecl *CurrentFunctionDecl);
+  ~TargetOMPContext() = default;
+

`virtual`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83281



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

[PATCH] D84244: [clang] Disable -Wsuggest-override for unittests/

[Logan Smith via Phabricator via cfe-commits]

logan-5 added a comment.

In D84244#2169142 , @hans wrote:

> Looks good to me so far. We haven't tried doing any full builds yet, but I 
> checked for the specific problem I hit yesterday (building 
> ClangApplyReplacementsTests) and that's working fine now.


Glad to hear that. I'll be keeping my eye on things today, but they look good 
so far to me too.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84244



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

[PATCH] D83281: [OpenMP] Allow traits for the OpenMP context selector `isa`

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert updated this revision to Diff 280151.
jdoerfert marked an inline comment as done.
jdoerfert added a comment.

Add virtual


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83281

Files:
  clang/include/clang/AST/OpenMPClause.h
  clang/include/clang/Basic/DiagnosticParseKinds.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/AST/OpenMPClause.cpp
  clang/lib/Parse/ParseOpenMP.cpp
  clang/lib/Sema/SemaOpenMP.cpp
  clang/test/OpenMP/declare_variant_device_isa_codegen_1.c
  clang/test/OpenMP/declare_variant_messages.c
  llvm/include/llvm/Frontend/OpenMP/OMPContext.h
  llvm/include/llvm/Frontend/OpenMP/OMPKinds.def
  llvm/lib/Frontend/OpenMP/OMPContext.cpp
  llvm/unittests/Frontend/OpenMPContextTest.cpp

Index: llvm/unittests/Frontend/OpenMPContextTest.cpp
===
--- llvm/unittests/Frontend/OpenMPContextTest.cpp
+++ llvm/unittests/Frontend/OpenMPContextTest.cpp
@@ -38,11 +38,13 @@
 #define OMP_TRAIT_PROPERTY(Enum, TraitSetEnum, TraitSelectorEnum, Str) \
   EXPECT_EQ(TraitProperty::Enum,   \
 getOpenMPContextTraitPropertyKind( \
-TraitSet::TraitSetEnum,\
-getOpenMPContextTraitPropertyName(TraitProperty::Enum)));  \
+TraitSet::TraitSetEnum, TraitSelector::TraitSelectorEnum,  \
+getOpenMPContextTraitPropertyName(TraitProperty::Enum, Str))); \
   EXPECT_EQ(Str, getOpenMPContextTraitPropertyName(\
- getOpenMPContextTraitPropertyKind(TraitSet::TraitSetEnum, \
-   Str))); \
+ getOpenMPContextTraitPropertyKind(\
+ TraitSet::TraitSetEnum,   \
+ TraitSelector::TraitSelectorEnum, Str),   \
+ Str));\
   EXPECT_EQ(TraitSet::TraitSetEnum,\
 getOpenMPContextTraitSetForProperty(TraitProperty::Enum)); \
   EXPECT_EQ(TraitSelector::TraitSelectorEnum,  \
Index: llvm/lib/Frontend/OpenMP/OMPContext.cpp
===
--- llvm/lib/Frontend/OpenMP/OMPContext.cpp
+++ llvm/lib/Frontend/OpenMP/OMPContext.cpp
@@ -175,11 +175,11 @@
 LLVM_DEBUG({
   if (MK == MK_ALL)
 dbgs() << "[" << DEBUG_TYPE << "] Property "
-   << getOpenMPContextTraitPropertyName(Property)
+   << getOpenMPContextTraitPropertyName(Property, "")
<< " was not in the OpenMP context but match kind is all.\n";
   if (MK == MK_NONE)
 dbgs() << "[" << DEBUG_TYPE << "] Property "
-   << getOpenMPContextTraitPropertyName(Property)
+   << getOpenMPContextTraitPropertyName(Property, "")
<< " was in the OpenMP context but match kind is none.\n";
 });
 return false;
@@ -198,6 +198,14 @@
   continue;
 
 bool IsActiveTrait = Ctx.ActiveTraits.test(unsigned(Property));
+
+// We overwrite the isa trait as it is actually up to the OMPContext hook to
+// check the raw string(s).
+if (Property == TraitProperty::device_isa___ANY)
+  IsActiveTrait = llvm::all_of(VMI.ISATraits, [&](StringRef RawString) {
+return Ctx.matchesISATrait(RawString);
+  });
+
 Optional Result = HandleTrait(Property, IsActiveTrait);
 if (Result.hasValue())
   return Result.getValue();
@@ -225,7 +233,7 @@
 
   if (!FoundInOrder) {
 LLVM_DEBUG(dbgs() << "[" << DEBUG_TYPE << "] Construct property "
-  << getOpenMPContextTraitPropertyName(Property)
+  << getOpenMPContextTraitPropertyName(Property, "")
   << " was not nested properly.\n");
 return false;
   }
@@ -425,8 +433,12 @@
   llvm_unreachable("Unknown trait selector!");
 }
 
-TraitProperty llvm::omp::getOpenMPContextTraitPropertyKind(TraitSet Set,
-   StringRef S) {
+TraitProperty llvm::omp::getOpenMPContextTraitPropertyKind(
+TraitSet Set, TraitSelector Selector, StringRef S) {
+  // Special handling for `device={isa(...)}` as we accept anything here. It is
+  // up to the target to decide if the feature is available.
+  if (Set == TraitSet::device && Selector == TraitSelector::device_isa)
+return TraitProperty::device_isa___ANY;
 #define OMP_TRAIT_PROPERTY(Enum, TraitSetEnum, TraitSelectorEnum, Str) \
   if (Set == TraitSet::TraitSetEnum && Str == S)   \
 return TraitProperty

[clang] 831ae45 - Recommit "[libFuzzer] Link libFuzzer's own interceptors when other compiler runtimes are not linked."

[Dokyung Song via cfe-commits]

Author: Dokyung Song
Date: 2020-07-23T15:59:07Z
New Revision: 831ae45e3dc609e43ba561af07670a8fe47461ef

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

LOG: Recommit "[libFuzzer] Link libFuzzer's own interceptors when other 
compiler runtimes are not linked."

Summary: libFuzzer intercepts certain library functions such as memcmp/strcmp 
by defining weak hooks. Weak hooks, however, are called only when other 
runtimes such as ASan is linked. This patch defines libFuzzer's own 
interceptors, which is linked into the libFuzzer executable when other runtimes 
are not linked, i.e., when -fsanitize=fuzzer is given, but not others.

The patch once landed but was reverted in 
8ef9e2bf355d05bc81d8b0fe1e5333eec59a0a91 due to an assertion failure caused by 
calling an intercepted function, strncmp, while initializing the interceptors 
in fuzzerInit(). This issue is now fixed by calling libFuzzer's own 
implementation of library functions (i.e., internal_*) when the fuzzer has not 
been initialized yet, instead of recursively calling fuzzerInit() again.

Reviewers: kcc, morehouse, hctim

Subscribers: #sanitizers, krytarowski, mgorny, cfe-commits

Tags: #clang, #sanitizers

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

Added: 
compiler-rt/lib/fuzzer/FuzzerInterceptors.cpp
compiler-rt/test/fuzzer/CustomAllocator.cpp
compiler-rt/test/fuzzer/custom-allocator.test

Modified: 
clang/include/clang/Driver/SanitizerArgs.h
clang/lib/Driver/SanitizerArgs.cpp
clang/lib/Driver/ToolChains/CommonArgs.cpp
compiler-rt/lib/fuzzer/CMakeLists.txt
compiler-rt/test/fuzzer/memcmp.test
compiler-rt/test/fuzzer/memcmp64.test
compiler-rt/test/fuzzer/strcmp.test
compiler-rt/test/fuzzer/strncmp.test
compiler-rt/test/fuzzer/strstr.test

Removed: 




diff  --git a/clang/include/clang/Driver/SanitizerArgs.h 
b/clang/include/clang/Driver/SanitizerArgs.h
index 934dab808e82..563d6c3ff9de 100644
--- a/clang/include/clang/Driver/SanitizerArgs.h
+++ b/clang/include/clang/Driver/SanitizerArgs.h
@@ -74,6 +74,7 @@ class SanitizerArgs {
!Sanitizers.has(SanitizerKind::Address) &&
!Sanitizers.has(SanitizerKind::HWAddress);
   }
+  bool needsFuzzerInterceptors() const;
   bool needsUbsanRt() const;
   bool requiresMinimalRuntime() const { return MinimalRuntime; }
   bool needsDfsanRt() const { return Sanitizers.has(SanitizerKind::DataFlow); }

diff  --git a/clang/lib/Driver/SanitizerArgs.cpp 
b/clang/lib/Driver/SanitizerArgs.cpp
index bcc9ffc7ff8f..e4fda752c041 100644
--- a/clang/lib/Driver/SanitizerArgs.cpp
+++ b/clang/lib/Driver/SanitizerArgs.cpp
@@ -240,6 +240,10 @@ static SanitizerMask parseSanitizeTrapArgs(const Driver &D,
   return TrappingKinds;
 }
 
+bool SanitizerArgs::needsFuzzerInterceptors() const {
+  return needsFuzzer() && !needsAsanRt() && !needsTsanRt() && !needsMsanRt();
+}
+
 bool SanitizerArgs::needsUbsanRt() const {
   // All of these include ubsan.
   if (needsAsanRt() || needsMsanRt() || needsHwasanRt() || needsTsanRt() ||

diff  --git a/clang/lib/Driver/ToolChains/CommonArgs.cpp 
b/clang/lib/Driver/ToolChains/CommonArgs.cpp
index 6b6e276b8ce7..acde6d9e2111 100644
--- a/clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ b/clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -784,6 +784,9 @@ bool tools::addSanitizerRuntimes(const ToolChain &TC, const 
ArgList &Args,
   !Args.hasArg(options::OPT_shared)) {
 
 addSanitizerRuntime(TC, Args, CmdArgs, "fuzzer", false, true);
+if (SanArgs.needsFuzzerInterceptors())
+  addSanitizerRuntime(TC, Args, CmdArgs, "fuzzer_interceptors", false,
+  true);
 if (!Args.hasArg(clang::driver::options::OPT_nostdlibxx))
   TC.AddCXXStdlibLibArgs(Args, CmdArgs);
   }

diff  --git a/compiler-rt/lib/fuzzer/CMakeLists.txt 
b/compiler-rt/lib/fuzzer/CMakeLists.txt
index b5be6b89452e..02be89cb70a5 100644
--- a/compiler-rt/lib/fuzzer/CMakeLists.txt
+++ b/compiler-rt/lib/fuzzer/CMakeLists.txt
@@ -99,6 +99,13 @@ add_compiler_rt_object_libraries(RTfuzzer_main
   CFLAGS ${LIBFUZZER_CFLAGS}
   DEPS ${LIBFUZZER_DEPS})
 
+add_compiler_rt_object_libraries(RTfuzzer_interceptors
+  OS ${FUZZER_SUPPORTED_OS}
+  ARCHS ${FUZZER_SUPPORTED_ARCH}
+  SOURCES FuzzerInterceptors.cpp
+  CFLAGS ${LIBFUZZER_CFLAGS}
+  DEPS ${LIBFUZZER_DEPS})
+
 add_compiler_rt_runtime(clang_rt.fuzzer
   STATIC
   OS ${FUZZER_SUPPORTED_OS}
@@ -115,6 +122,14 @@ add_compiler_rt_runtime(clang_rt.fuzzer_no_main
   CFLAGS ${LIBFUZZER_CFLAGS}
   PARENT_TARGET fuzzer)
 
+add_compiler_rt_runtime(clang_rt.fuzzer_interceptors
+  STATIC
+  OS ${FUZZER_SUPPORTED_OS}
+  ARCHS ${FUZZER_SUPPORTED_ARCH}
+  OBJECT_LIBS RTfuzzer_interceptors
+  CFLAGS ${LIBFUZZER_CFLAGS}
+  PARENT_TARGET fuzzer)
+
 if(OS_NAME MATCHES "Linux|Fuchsia" AND

[PATCH] D83494: [libFuzzer] Link libFuzzer's own interceptors when other compiler runtimes are not linked.

[Dokyung Song via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
dokyungs marked an inline comment as done.
Closed by commit rG831ae45e3dc6: Recommit "[libFuzzer] Link 
libFuzzer's own interceptors when other compiler… (authored by dokyungs).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83494

Files:
  clang/include/clang/Driver/SanitizerArgs.h
  clang/lib/Driver/SanitizerArgs.cpp
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  compiler-rt/lib/fuzzer/CMakeLists.txt
  compiler-rt/lib/fuzzer/FuzzerInterceptors.cpp
  compiler-rt/test/fuzzer/CustomAllocator.cpp
  compiler-rt/test/fuzzer/custom-allocator.test
  compiler-rt/test/fuzzer/memcmp.test
  compiler-rt/test/fuzzer/memcmp64.test
  compiler-rt/test/fuzzer/strcmp.test
  compiler-rt/test/fuzzer/strncmp.test
  compiler-rt/test/fuzzer/strstr.test

Index: compiler-rt/test/fuzzer/strstr.test
===
--- compiler-rt/test/fuzzer/strstr.test
+++ compiler-rt/test/fuzzer/strstr.test
@@ -1,5 +1,11 @@
 UNSUPPORTED: freebsd
 RUN: %cpp_compiler %S/StrstrTest.cpp -o %t-StrstrTest
 RUN: not %run %t-StrstrTest   -seed=1 -runs=200   2>&1 | FileCheck %s
-CHECK: BINGO
 
+RUN: %cpp_compiler -fno-sanitize=address -fno-builtin-strstr %S/StrstrTest.cpp -o %t-NoAsanStrstrTest
+RUN: not %run %t-NoAsanStrstrTest -seed=1 -runs=200   2>&1 | FileCheck %s
+
+RUN: %cpp_compiler -fno-sanitize=address -fno-builtin-calloc -fno-builtin-strstr %S/CustomAllocator.cpp %S/StrstrTest.cpp -o %t-NoAsanCustomAllocatorStrstrTest
+RUN: not %run %t-NoAsanCustomAllocatorStrstrTest -seed=1 -runs=200   2>&1 | FileCheck %s
+
+CHECK: BINGO
Index: compiler-rt/test/fuzzer/strncmp.test
===
--- compiler-rt/test/fuzzer/strncmp.test
+++ compiler-rt/test/fuzzer/strncmp.test
@@ -1,5 +1,11 @@
 UNSUPPORTED: freebsd
 RUN: %cpp_compiler %S/StrncmpTest.cpp -o %t-StrncmpTest
 RUN: not %run %t-StrncmpTest  -seed=2 -runs=1000   2>&1 | FileCheck %s
-CHECK: BINGO
 
+RUN: %cpp_compiler -fno-sanitize=address -fno-builtin-strncmp %S/StrncmpTest.cpp -o %t-NoAsanStrncmpTest
+RUN: not %run %t-NoAsanStrncmpTest-seed=2 -runs=1000   2>&1 | FileCheck %s
+
+RUN: %cpp_compiler -fno-sanitize=address -fno-builtin-calloc -fno-builtin-strncmp %S/CustomAllocator.cpp %S/StrncmpTest.cpp -o %t-NoAsanCustomAllocatorStrncmpTest
+RUN: not %run %t-NoAsanCustomAllocatorStrncmpTest -seed=2 -runs=1000   2>&1 | FileCheck %s
+
+CHECK: BINGO
Index: compiler-rt/test/fuzzer/strcmp.test
===
--- compiler-rt/test/fuzzer/strcmp.test
+++ compiler-rt/test/fuzzer/strcmp.test
@@ -1,5 +1,11 @@
 UNSUPPORTED: freebsd
 RUN: %cpp_compiler %S/StrcmpTest.cpp -o %t-StrcmpTest
 RUN: not %run %t-StrcmpTest   -seed=1 -runs=200   2>&1 | FileCheck %s
-CHECK: BINGO
 
+RUN: %cpp_compiler -fno-sanitize=address -fno-builtin-strcmp %S/StrcmpTest.cpp -o %t-NoAsanStrcmpTest
+RUN: not %run %t-NoAsanStrcmpTest -seed=1 -runs=200   2>&1 | FileCheck %s
+
+RUN: %cpp_compiler -fno-sanitize=address -fno-builtin-calloc -fno-builtin-strcmp %S/CustomAllocator.cpp %S/StrcmpTest.cpp -o %t-NoAsanCustomAllocatorStrcmpTest
+RUN: not %run %t-NoAsanCustomAllocatorStrcmpTest -seed=1 -runs=200   2>&1 | FileCheck %s
+
+CHECK: BINGO
Index: compiler-rt/test/fuzzer/memcmp64.test
===
--- compiler-rt/test/fuzzer/memcmp64.test
+++ compiler-rt/test/fuzzer/memcmp64.test
@@ -1,4 +1,8 @@
 UNSUPPORTED: freebsd
 RUN: %cpp_compiler %S/Memcmp64BytesTest.cpp -o %t-Memcmp64BytesTest
 RUN: not %run %t-Memcmp64BytesTest-seed=1 -runs=100   2>&1 | FileCheck %s
+
+RUN: %cpp_compiler -fno-sanitize=address -fno-builtin-memcmp %S/Memcmp64BytesTest.cpp -o %t-NoAsanMemcmp64BytesTest
+RUN: not %run %t-NoAsanMemcmp64BytesTest  -seed=1 -runs=100   2>&1 | FileCheck %s
+
 CHECK: BINGO
Index: compiler-rt/test/fuzzer/memcmp.test
===
--- compiler-rt/test/fuzzer/memcmp.test
+++ compiler-rt/test/fuzzer/memcmp.test
@@ -1,4 +1,11 @@
 UNSUPPORTED: freebsd
 RUN: %cpp_compiler %S/MemcmpTest.cpp -o %t-MemcmpTest
 RUN: not %run %t-MemcmpTest   -seed=1 -runs=1000   2>&1 | FileCheck %s
+
+RUN: %cpp_compiler -fno-sanitize=address -fno-builtin-memcmp %S/MemcmpTest.cpp -o %t-NoAsanMemcmpTest
+RUN: not %run %t-NoAsanMemcmpTest -seed=1 -runs=1000   2>&1 | FileCheck %s
+
+RUN: %cpp_compiler -fno-sanitize=address -fno-builtin-calloc -fno-builtin-memcmp %S/CustomAllocator.cpp %S/MemcmpTest.cpp -o %t-NoAsanCustomAllocatorMemcmpTest
+RUN: not %run %t-NoAsanCustomAllocatorMemcmpTest -seed=1 -runs=1000   2>&1 | FileCheck %s
+
 CHECK: BINGO
Index: compiler-rt/test/fuzzer/custom-allocator.test

[PATCH] D84414: [RISCV] Support Shadow Call Stack

[Ana Pazos via Phabricator via cfe-commits]

apazos added inline comments.



Comment at: llvm/lib/Target/RISCV/RISCVFrameLowering.cpp:95
+  .addReg(RISCV::X18)
+  .addImm(0);
+}

There are thee things to observe here and other reviewers might have some 
additional comments:

- RISC-V does not have a reserved platform register like AAch64. The patch uses 
one of the RISC-V callee saved registers, x18, which happens to coincide with 
AArch64's register. It is possible to select another register, and additional 
checks for the flag combo "-fsanitize=shadow-call-stack -ffixed-" will have 
to be added.

- The  return address is saved on both the SCS (whose location is 
protected/hidden) and also in the regular stack. But the return from a function 
uses the value saved on SCS. The understanding is that not saving it in the 
regular stack can impact debugging.

- The SCS is ascending, while the regular stack, by RISC-V convention, is 
descending. The SCS is not used for passing parameters between calls like the 
regular stack, so it seems to be ok. But this can be changed too. AArch64 's 
SCS is also ascending.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84414



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

[PATCH] D84422: [OpenMP] Fix `present` for exit from `omp target data`

[Joel E. Denny via Phabricator via cfe-commits]

jdenny created this revision.
jdenny added reviewers: grokos, ABataev, jdoerfert.
Herald added subscribers: llvm-commits, openmp-commits, cfe-commits, sstefan1, 
guansong, yaxunl.
Herald added projects: clang, OpenMP, LLVM.

Without this patch, the following example fails but shouldn't
according to my read of OpenMP TR8:

  #pragma omp target enter data map(alloc:i)
  #pragma omp target data map(present, alloc: i)
  {
#pragma omp target exit data map(delete:i)
  } // fails presence check here

OpenMP TR8 sec. 2.22.7.1 "map Clause", p. 321, L23-26 states:

> If the map clause appears on a target, target data, target enter
>  data or target exit data construct with a present map-type-modifier
>  then on entry to the region if the corresponding list item does not 
>  appear in the device data environment an error occurs and the 
>  program terminates.

I see no corresponding statement about the exit from a region.  Thus,
the `present` modifier should:

1. Check for presence upon entry into a `target exit data` construct.
2. Should not check for presence upon exit from a `target data` region, as in 
the above example.

The problem is that Clang calls the same set of
`__tgt_target_data_end_*` functions for these two cases, making them
indistinguishable in the runtime where the presence check is
implemented.  To fix that, this patch changes Clang to generate calls
to a new set of runtime functions, `__tgt_target_exit_data_*`, for the 
case of `target exit data`.

For symmetry, this patch makes a similar change for `target enter
data`, but that change isn't required for the above fix.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D84422

Files:
  clang/lib/CodeGen/CGOpenMPRuntime.cpp
  clang/test/OpenMP/declare_mapper_codegen.cpp
  clang/test/OpenMP/target_enter_data_codegen.cpp
  clang/test/OpenMP/target_enter_data_depend_codegen.cpp
  clang/test/OpenMP/target_exit_data_codegen.cpp
  clang/test/OpenMP/target_exit_data_depend_codegen.cpp
  clang/test/OpenMP/target_map_member_expr_array_section_codegen.cpp
  llvm/include/llvm/Frontend/OpenMP/OMPKinds.def
  openmp/libomptarget/include/omptarget.h
  openmp/libomptarget/src/exports
  openmp/libomptarget/src/interface.cpp
  openmp/libomptarget/src/omptarget.cpp
  openmp/libomptarget/src/private.h
  openmp/libomptarget/test/mapping/present/target_data_at_exit.c

Index: openmp/libomptarget/test/mapping/present/target_data_at_exit.c
===
--- /dev/null
+++ openmp/libomptarget/test/mapping/present/target_data_at_exit.c
@@ -0,0 +1,37 @@
+// RUN: %libomptarget-compile-aarch64-unknown-linux-gnu -fopenmp-version=51
+// RUN: %libomptarget-run-aarch64-unknown-linux-gnu 2>&1 \
+// RUN: | %fcheck-aarch64-unknown-linux-gnu
+
+// RUN: %libomptarget-compile-powerpc64-ibm-linux-gnu -fopenmp-version=51
+// RUN: %libomptarget-run-powerpc64-ibm-linux-gnu 2>&1 \
+// RUN: | %fcheck-powerpc64-ibm-linux-gnu
+
+// RUN: %libomptarget-compile-powerpc64le-ibm-linux-gnu -fopenmp-version=51
+// RUN: %libomptarget-run-powerpc64le-ibm-linux-gnu 2>&1 \
+// RUN: | %fcheck-powerpc64le-ibm-linux-gnu
+
+// RUN: %libomptarget-compile-x86_64-pc-linux-gnu -fopenmp-version=51
+// RUN: %libomptarget-run-x86_64-pc-linux-gnu 2>&1 \
+// RUN: | %fcheck-x86_64-pc-linux-gnu
+
+#include 
+
+int main() {
+  int i;
+
+#pragma omp target enter data map(alloc:i)
+
+  // i isn't present at the end of the target data region, but the "present"
+  // modifier is only checked at the beginning of a region.
+#pragma omp target data map(present, alloc: i)
+  {
+#pragma omp target exit data map(delete:i)
+  }
+
+  // CHECK-NOT: Libomptarget
+  // CHECK: success
+  // CHECK-NOT: Libomptarget
+  fprintf(stderr, "success\n");
+
+  return 0;
+}
Index: openmp/libomptarget/src/private.h
===
--- openmp/libomptarget/src/private.h
+++ openmp/libomptarget/src/private.h
@@ -24,8 +24,8 @@
 
 extern int target_data_end(DeviceTy &Device, int32_t arg_num, void **args_base,
void **args, int64_t *arg_sizes, int64_t *arg_types,
-   void **arg_mappers,
-   __tgt_async_info *async_info_ptr);
+   void **arg_mappers, __tgt_async_info *async_info_ptr,
+   bool for_exit_data);
 
 extern int target_data_update(DeviceTy &Device, int32_t arg_num,
   void **args_base, void **args,
Index: openmp/libomptarget/src/omptarget.cpp
===
--- openmp/libomptarget/src/omptarget.cpp
+++ openmp/libomptarget/src/omptarget.cpp
@@ -421,10 +421,30 @@
   return OFFLOAD_SUCCESS;
 }
 
+static int target_data_end_not_for_exit_data(DeviceTy &Device, int32_t arg_num,
+ void **args_base, void **args,
+ int64_t *arg_sizes,
+   

[PATCH] D83997: [os_log] Improve the way we extend the lifetime of objects passed to __builtin_os_log_format

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak added a comment.

The use case for this is a macro in which the call to `__builtin_os_log_format` 
that writes to the buffer and the call that uses the buffer appear in two 
different statements. For example:

  __builtin_os_log_format(buf, "%@", getObj());
  ...
  use_buffer(buf);

The object returned by the call to `getObj` has to be kept alive until 
`use_buffer` is called, but currently it gets destructed at the end of the full 
expression. I think an alternate solution would be to provide users a means to 
tell ARC optimizer not to move the release call for a local variable past any 
calls, i.e., something that is stricter than `NS_VALID_UNTIL_END_OF_SCOPE`, but 
that places more burden on the users.

In the `os_log` macro, the result of the call to `__builtin_os_log_format` is 
passed directly to the call that uses the buffer, so it doesn't require any 
lifetime extension as you pointed out.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83997



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

[PATCH] D84310: [libTooling] Add assorted `EditGenerator` combinators.

[Andy Soffer via Phabricator via cfe-commits]

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



Comment at: clang/include/clang/Tooling/Transformer/RewriteRule.h:125
+/// not bound, then no edits are produced.
+inline EditGenerator ifBound(std::string ID, ASTEdit TrueEdit) {
+  return ifBound(std::move(ID), edit(std::move(TrueEdit)), noEdits());

I don't know about LLVM style preferences here, but a default argument to the 
example above seems reasonable too.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84310



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

[PATCH] D79744: clang: Use byref for aggregate kernel arguments

[Matt Arsenault via Phabricator via cfe-commits]

arsenm added a comment.

In D79744#2165929 , @rjmccall wrote:

> Arguably we should add this attribute to all indirect arguments.  I can 
> understand not wanting to update all the test cases, but you could probably 
> avoid adding a new IndirectByRef kind of ABIArgInfo by treating kernels 
> specially in ConstructAttributeList.
>
> Or, sorry, I forget — is this semantically necessary because the argument is 
> to constant memory and the callee has to copy it to form the mutable local?  
> If so, I think (1) the above statement about theoretically using `byref` on 
> all arguments still applies and (2) we do need a new ABIArgInfo kind, but we 
> should name it something like IndirectAliased.


Yes, it's semantically needed to insert the copy from constant memory. I 
originally interpreted a copy as necessary if the indirect addrspace did not 
match the stack address space, which is a sort of roundabout way of achieving 
the same thing


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

https://reviews.llvm.org/D79744



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

[PATCH] D84310: [libTooling] Add assorted `EditGenerator` combinators.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

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



Comment at: clang/include/clang/Tooling/Transformer/RewriteRule.h:125
+/// not bound, then no edits are produced.
+inline EditGenerator ifBound(std::string ID, ASTEdit TrueEdit) {
+  return ifBound(std::move(ID), edit(std::move(TrueEdit)), noEdits());

asoffer wrote:
> I don't know about LLVM style preferences here, but a default argument to the 
> example above seems reasonable too.
I'd like that, but don't see how to get the types to work out, "noEdits" is a 
generator, but FalseEdit branch is an `ASTEdit`. did you have something in mind?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84310



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

[PATCH] D79279: Add overloaded versions of builtin mem* functions

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

In D79279#2169522 , @jfb wrote:

> In D79279#2168649 , @rjmccall wrote:
>
> > In D79279#2168533 , @jfb wrote:
> >
> > > In D79279#2168479 , @rjmccall 
> > > wrote:
> > >
> > > > Is there a need for an atomic memcpy at all?  Why is it useful to allow 
> > > > this operation to take on "atomic" semantics — which aren't actually 
> > > > atomic because the loads and stores to elements are torn — with 
> > > > hardcoded memory ordering and somewhat arbitrary rules about what the 
> > > > atomic size is?
> > >
> > >
> > > Hans lays out a rationale for usefulness in his paper, but what I've 
> > > implemented is more useful: it's *unordered* so you can fence as you 
> > > desire around it, yet it guarantees a minimum memory access size based on 
> > > the pointer parameters. For example, copying an atomic `int` will be 4 
> > > byte operations which are single-copy-atomic, but the accesses from one 
> > > `int` to the next aren't performed in any guaranteed order (or observable 
> > > in any guaranteed order either). I talked about this with him a while ago 
> > > but IIRC he wasn't sure about implementation among other things, so when 
> > > you asked me to widen my original `volatile`-only `memcpy` to also do 
> > > other qualifiers, I realized that it was a neat way to do atomic as well 
> > > as other qualifiers. I've talked to a few SG1 folks about this, and I 
> > > believe (for other reasons too) it's where the design will end up for 
> > > Hans' paper.
> >
> >
> > I can see the usefulness of this operation, but it seems like a odd 
> > semantic mismatch for what is basically just a memcpy where one of the 
> > pointers happens to have `_Atomic` type, like you're shoe-horning it into 
> > this builtin just to avoid declaring a different one.
>
>
> I'm following the discussion we had here regarding overloading 
> :
>
> >> There are other qualifiers that can meaningfully contribute to the 
> >> operation here besides volatile, such as restrict and (more importantly) 
> >> address spaces. And again, for the copy operations these might differ 
> >> between the two pointer types.
> >> 
> >> In both cases, I’d say that the logical design is to allow the pointers to 
> >> be to arbitrarily-qualified types. We can then propagate that information 
> >> from the builtin into the LLVM intrinsic call as best as we’re allowed. So 
> >> I think you should make builtins called something like 
> >> __builtin_overloaded_memcpy (name to be decided) and just have their 
> >> semantics be type-directed.
> > 
> > Ah yes, I’d like to hear what others think of this. I hadn’t thought about 
> > it before you brought it up, and it sounds like a good idea.
>
> As you noted earlier, for `memcpy` you probably want to express differences 
> in destination and source qualification, even if today IR can't express e.g. 
> `volatile` source and non-`volatile` destination. You were talking about 
> `volatile`, but this applies to the entire combination of dst+src qualified 
> with zero-to-five `volatile`, `_Atomic`, `__unaligned`, `restrict`, and 
> address space. Pulling the entire combination space out into different 
> functions would create way too many functions. Right now the implementation 
> has a few limitations: it treats both dst and src as `volatile` if either 
> are, it can't do `_Atomic` with `volatile` so we diagnose, and it ignores 
> `restrict`.  Otherwise it supports all combinations.


My point is that this has nothing to do with the ordinary semantics of 
`_Atomic`.  You're basically just looking at the word "atomic" and saying that, 
hey, a minimum access size is sortof related to atomicity.

If you want this to be able to control the minimum access size, you should 
allow that to be passed in as an optional argument instead.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79279



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

[PATCH] D83553: [PATCH 3/4][Sema][AArch64] Add codegen for arm_sve_vector_bits attribute

[Cullen Rhodes via Phabricator via cfe-commits]

c-rhodes marked an inline comment as done.
c-rhodes added inline comments.



Comment at: clang/lib/CodeGen/CGExpr.cpp:152
+  Align, Name,
+  /*ArraySize=*/nullptr, Alloca);
 

efriedma wrote:
> Do we need to bitcast the result of CreateTempAlloca to a pointer to the 
> array type?  I'm concerned that we might miss a bitcast if the source code 
> uses the address of the variable.
> Do we need to bitcast the result of CreateTempAlloca to a pointer to the 
> array type? I'm concerned that we might miss a bitcast if the source code 
> uses the address of the variable.

You were right, I've spent some time investigating this. The current 
implementation crashes on:
```fixed_int32_t global;
fixed_int32_t address_of_global() {
  fixed_int32_t *global_ptr;
  global_ptr = &global;
  return *global_ptr;
}```

the reason being `global` is represented as an `ArrayType` whereas the pointer 
`global_ptr` is scalable:

```@global = global [4 x i32] zeroinitializer, align 16
%global_ptr = alloca *, align 8```

so when storing the address of `global` to `global_ptr` the store it tries to 
create causes a crash:

`store [4 x i32]* @global, ** %global_ptr, align 8`

I tried your suggestion to bitcast to alloca to the array type in 
`CreateMemTemp` but found for that example it isn't called, it's created by a 
call to `CreateTempAlloca` in CGDecl.cpp (`EmitAutoVarAlloca`). 
`CreateTempAlloca` takes an `llvm::Type *Ty` so it's not as straightforward as 
doing a bitcast there, although I found it could be done in `EmitAutoVarAlloca` 
but it means having to handle this is two places I'm aware of and potentially 
others I haven't hit. In this case as well it also required looking through the 
pointer to see if the pointee was a VLST then doing a bitcast.

I've also experimented with representing allocas as fixed-length arrays to see 
if that will make it any easier and it does simplify the patch a little. It 
does require handling `PointerType` in `ConvertTypeForMem` however as we do for 
`ConstantArray`, same issue I mentioned in response to your other comment about 
removing that.

I planning to update the patch with that implementation but I've just found 
another issue:

```fixed_int32_t arr[3];
fixed_int32_t *z() {
  fixed_int32_t *array_ptr;
  array_ptr = &arr[0];
  return array_ptr;
}```

trying to create a store:
`store [4 x i32]* %0, ** %retval, align 8`

although this is done in CGStmt.cpp as it's for a retval so it looks like a 
bitcast could also be required there.



Comment at: clang/lib/CodeGen/CodeGenModule.cpp:3985
+else
+  Init = EmitNullConstant(D->getType());
   } else {

efriedma wrote:
> EmitNullConstant should just do the right thing, I think, now that we've 
> changed the default behavior of ConvertTypeForMem.
> EmitNullConstant should just do the right thing, I think, now that we've 
> changed the default behavior of ConvertTypeForMem.

Good spot, these changes can be removed



Comment at: clang/lib/CodeGen/CodeGenTypes.cpp:151
+  return llvm::ArrayType::get(*MemTy, A->getSize().getZExtValue());
+  }
+

efriedma wrote:
> I think the default handling for constant arrays should do the right thing, 
> now that we've changed the default behavior of ConvertTypeForMem.
> I think the default handling for constant arrays should do the right thing, 
> now that we've changed the default behavior of ConvertTypeForMem.

`ConvertType` looks at the canonical type so the type attribute is lost.


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

https://reviews.llvm.org/D83553



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

[PATCH] D79279: Add overloaded versions of builtin mem* functions

[JF Bastien via Phabricator via cfe-commits]

jfb added a comment.

> My point is that this has nothing to do with the ordinary semantics of 
> `_Atomic`.  You're basically just looking at the word "atomic" and saying 
> that, hey, a minimum access size is sortof related to atomicity.
> 
> If you want this to be able to control the minimum access size, you should 
> allow that to be passed in as an optional argument instead.

OK so it sounds like you're suggesting *two* versions of the overloaded 
builtins:

1. `__builtin_memcpy_overloaded` which overloads on `volatile`, `restrict`, 
`__unaligned`, and address spaces, but **not** on `_Atomic` qualifiers.
2. `__builtin_atomic_memcpy_overloaded` which overloads on `volatile` (but 
unsupported for now), `restrict`, and address spaces, but **not** on `_Atomic` 
qualifiers (because it's implicit), and **not** on `__unaligned` because that's 
a constraint. This takes an extra "element size" parameter, which we hope folks 
don't confuse with the size parameter (I'd expect a template or macro wrapper 
to hide that extra parameter when actually using the builtin).

Of course, that's two versions for each of `memcpy`, `memmove`, `memset`, and 
any other `*mem` that we decide to add to this list of overloadable functions.

Is that correct?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79279



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

[PATCH] D75044: [AArch64] __builtin_return_address for PAuth.

[Momchil Velikov via Phabricator via cfe-commits]

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

LGTM. Thanks!


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

https://reviews.llvm.org/D75044



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

[PATCH] D79279: Add overloaded versions of builtin mem* functions

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

I don't think any of these should allow _Atomic unless we're going to give it 
some sort of consistent atomic semantics (which is hard to imagine being 
useful), and I think you should just take an extra argument of the minimum 
access width on all of them uniformly if you think that's important.  Builtins 
can have optional arguments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79279



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

[PATCH] D84029: [clang][Driver] Default to /usr/bin/ld on Solaris

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

Can you add a test to `test/Driver/solaris-ld.c`?

> However, if someone forgets this, it depends on the user's PATH whether or 
> not clang finds the correct linker, which doesn't make for a good user 
> experience.

Not very sure about this. The last resort of GetProgramPath is `PATH`. On 
FreeBSD and Linux, this is how `/usr/bin/ld` gets selected. PATH can affect 
`ld` choice.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84029



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

[PATCH] D84405: CodeGen: Improve generated IR for __builtin_mul_overflow(uint, uint, int)

[Vedant Kumar via Phabricator via cfe-commits]

vsk added a comment.

How were you able to show that the specialized IRGen is equivalent to the 
generic kind? I tried doing this with direct inspection 
(https://godbolt.org/z/o5WEn3), but wasn't able to convince myself. In the past 
I used a test driver to compare the before/after compiler output -- you might 
find that useful 
(https://gist.github.com/vedantk/3eb9c88f82e5c32f2e590555b4af5081).




Comment at: clang/test/CodeGen/builtins-overflow.c:123
+  // CHECK: br i1 [[C2]]
+  int r;
+  if (__builtin_mul_overflow(x, y, &r))

Could you add a test for the volatile result case?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84405



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

[PATCH] D79279: Add overloaded versions of builtin mem* functions

[JF Bastien via Phabricator via cfe-commits]

jfb added a comment.

In D79279#2170095 , @rjmccall wrote:

> I don't think any of these should allow _Atomic unless we're going to give it 
> some sort of consistent atomic semantics (which is hard to imagine being 
> useful), and I think you should just take an extra argument of the minimum 
> access width on all of them uniformly if you think that's important.  
> Builtins can have optional arguments.


OK so: `__builtin_memcpy_overloaded` which overloads on `volatile`, `restrict`, 
`__unaligned`, and address spaces, but not on `_Atomic` qualifiers. Optionally, 
a 4th integer parameter can be provided to represent `element_size`. If 
provided, this becomes an unordered atomic memcpy with element size equal to or 
greater than the provided `element_size`. That value must be a power of two, 
and must be lock-free (what we call maximum atomic inline width in target 
info). If provided, then `__unaligned` is invalid, and `volatile` ought to be 
valid but is currently unsupported because IR can't do atomic+`volatile` memcpy 
(it would be useful, say for shared memory, but Patches Welcome).

Do you think there should be any relationship at all between dst/src pointee 
type's size and `element_size`? i.e. if I copy `short*` using an element size 
of 1 byte, is that OK? It seems like larger element sizes is always OK, but 
smaller might be a programmer error? If that's what they wanted, they could 
have done `(char*)my_short`. Or is this trying to be too helpful?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79279



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

[PATCH] D79279: Add overloaded versions of builtin mem* functions

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

I think the argument is treated as if it were 1 if not given.  That's all that 
ordinary memcpy formally guarantees, which seems to work fine (semantically, if 
not performance-wise) for pretty much everything today.  I don't think you need 
any restrictions on element size.  It's probably sensible to require the 
pointers to be dynamically aligned to a multiple of the access width, but I 
don't think you can enforce that statically.  And of course the length needs to 
be a multiple of the access size.

Do you think it'd be useful to have different guarantees for different 
operands?  I guess it could come up, but it'd be a whole lot of extra 
complexity that I can't imagine we'd ever support.

If one of the arguments is `volatile`, arguably the minimum access width (if 
given) needs to be exact.  If we don't support that right now, it's okay to 
make it an error, which is basically you've already done with the `_Atomic 
volatile` diagnostic.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79279



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

[PATCH] D84316: [analyzer][NFC] Split CStringChecker to modeling and reporting

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

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

In D84316#2169195 , @Szelethus wrote:

> [...] you really cant make CStringChecker work without CStringModeling


How should I fuse the `CStringModeling` and the `CStringChecker` together while 
splitting them up?

I mean, that would be the best if the `CStringChecker` would focus on modeling 
the related cstring functions while letting the `CStringModeling` do the 
bookkeeping.
I see some contradiction here.




Comment at: clang/include/clang/StaticAnalyzer/Checkers/Checkers.td:429
 
 def CStringModeling : Checker<"CStringModeling">,
+  HelpText<"Responsible for essential modeling of cstring lengths. "

Szelethus wrote:
> What other aspects of c strings needs to be modeled? Is it only length? If 
> so, how about we rename the checker to `CStringLengthModeling`?
For now I think the cstring length is enough.
I'm not sure if we will want to have other properties as well.
You are probably right.



Comment at: clang/include/clang/StaticAnalyzer/Checkers/Checkers.td:495
   ]>,
-  Dependencies<[CStringModeling]>,
+  Dependencies<[CStringChecker]>,
   Documentation,

Szelethus wrote:
> I dug around a bit, and found this commit as to why this was needed: 
> rGe56167e8f87acf87a9de3d383752e18a738cf056. So this dependency is appropriate.
Interesting.
I was just doing a search & replace though :)



Comment at: clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp:74
 enum class ConcatFnKind { none = 0, strcat = 1, strlcat = 2 };
-class CStringChecker : public Checker< eval::Call,
- check::PreStmt,
- check::LiveSymbols,
- check::DeadSymbols,
- check::RegionChanges
- > {
+class CStringChecker : public Checker {
   mutable std::unique_ptr BT_Null, BT_Bounds, BT_Overlap,

Szelethus wrote:
> This is somewhat orthogonal to the patch, but shouldn't precondition 
> violations be reported at `preCall`?
That is the current behavior.
We should consider in the future using `preCall` if we refactor so relentlessly.



Comment at: clang/lib/StaticAnalyzer/Checkers/CStringLength.cpp:175-181
+void ento::registerCStringModeling(CheckerManager &Mgr) {
+  Mgr.registerChecker();
+}
+
+bool ento::shouldRegisterCStringModeling(const CheckerManager &) {
+  return true;
+}

Szelethus wrote:
> We traditionally put these on the bottom of the file -- I don't think this 
> would upset the structure too much :)
I wanted to place the class definition as close as possible to the registration 
function.
I can move this though.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84316



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

[PATCH] D84405: CodeGen: Improve generated IR for __builtin_mul_overflow(uint, uint, int)

[Tom Stellard via Phabricator via cfe-commits]

tstellar added a comment.

In D84405#2170110 , @vsk wrote:

> How were you able to show that the specialized IRGen is equivalent to the 
> generic kind? I tried doing this with direct inspection 
> (https://godbolt.org/z/o5WEn3), but wasn't able to convince myself. In the 
> past I used a test driver to compare the before/after compiler output -- you 
> might find that useful 
> (https://gist.github.com/vedantk/3eb9c88f82e5c32f2e590555b4af5081).


I compared with what gcc was generating and it looked similar (although my x86 
assembly knowledge is not great), but I will give your test driver a try, 
thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84405



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

[PATCH] D79279: Add overloaded versions of builtin mem* functions

[JF Bastien via Phabricator via cfe-commits]

jfb added a comment.

In D79279#2170157 , @rjmccall wrote:

> I think the argument is treated as if it were 1 if not given.  That's all 
> that ordinary memcpy formally guarantees, which seems to work fine 
> (semantically, if not performance-wise) for pretty much everything today.


I'm not sure that's true: consider a `memcpy` implementation which copies some 
bytes twice (at different access size, there's an overlap because somehow it's 
more efficient). That would probably violate the programmer's expectations, and 
I don't think `volatile` nor atomic `memcpy` allow this (but regular `memcpy` 
does).

> I don't think you need any restrictions on element size.  It's probably 
> sensible to require the pointers to be dynamically aligned to a multiple of 
> the access width, but I don't think you can enforce that statically.

Agreed, if we're given a short and told to copy 4 bytes at a time then UBSan 
could find the constraint violation on alignment, but generally the only way we 
can diagnose is if the parameter is `__unaligned` (because there you're 
explicitly telling me it's not aligned, and the constraint is that it has to 
be).

> And of course the length needs to be a multiple of the access size.

Yeah.

> Do you think it'd be useful to have different guarantees for different 
> operands?  I guess it could come up, but it'd be a whole lot of extra 
> complexity that I can't imagine we'd ever support.

You mean, if `element_size` is passed then you get different guarantees? I 
think that's what makes sense: if you're asking for atomic `memcpy` then you 
get guarantees. If you're asking for `volatile` `mempcy` then you get others. 
That's why overloading (and multiple parameters) can be confusing, but at the 
same time I think it's better than having the combinatorial number of named 
functions instead.

> If one of the arguments is `volatile`, arguably the minimum access width (if 
> given) needs to be exact.  If we don't support that right now, it's okay to 
> make it an error, which is basically you've already done with the `_Atomic 
> volatile` diagnostic.

Agreed. `volatile` with size makes a lot of sense, and the IR version of it, 
once created, ought to not be able to widen accesses. `volatile` without a size 
specified makes sense too, because you just want a single read and a single 
write, don't care about tearing.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79279



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

LLVM buildmaster is back to normal work

[Galina Kistanova via cfe-commits]

Hello everyone,

LLVM buildmaster is back to normal work, sorry for inconvenience.
Last night updates caused some distortions unfortunately.

Thanks

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

[PATCH] D84316: [analyzer][NFC] Split CStringChecker to modeling and reporting

[Kristóf Umann via Phabricator via cfe-commits]

Szelethus added a comment.

You could do it in the code, but if the modeling wouldn't be present from 
CStringModeling, CStringChecker wouldn't work properly. So you should make it a 
strong dependency, just as you did in this patch. My comment was mainly a 
response to @NoQ :)




Comment at: clang/lib/StaticAnalyzer/Checkers/CStringLength.cpp:175-181
+void ento::registerCStringModeling(CheckerManager &Mgr) {
+  Mgr.registerChecker();
+}
+
+bool ento::shouldRegisterCStringModeling(const CheckerManager &) {
+  return true;
+}

steakhal wrote:
> Szelethus wrote:
> > We traditionally put these on the bottom of the file -- I don't think this 
> > would upset the structure too much :)
> I wanted to place the class definition as close as possible to the 
> registration function.
> I can move this though.
Yeah, I see what you were going for, but I'd prefer to keep it down there still.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84316



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

[PATCH] D84364: [CUDA][HIP] Defer all diagnostics for host device functions

[Artem Belevich via Phabricator via cfe-commits]

tra added a comment.

I'm going to try the patch on our CUDA code and see how it fares. Stay tuned.


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

https://reviews.llvm.org/D84364



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

[PATCH] D79279: Add overloaded versions of builtin mem* functions

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

In D79279#2170187 , @jfb wrote:

> In D79279#2170157 , @rjmccall wrote:
>
> > I think the argument is treated as if it were 1 if not given.  That's all 
> > that ordinary memcpy formally guarantees, which seems to work fine 
> > (semantically, if not performance-wise) for pretty much everything today.
>
>
> I'm not sure that's true: consider a `memcpy` implementation which copies 
> some bytes twice (at different access size, there's an overlap because 
> somehow it's more efficient). That would probably violate the programmer's 
> expectations, and I don't think `volatile` nor atomic `memcpy` allow this 
> (but regular `memcpy` does).


Yes, that's true, if you need an only-accessed-once guarantee, that's above and 
beyond just a minimum access size.  I agree that `volatile` would need to make 
this guarantee.

>> Do you think it'd be useful to have different guarantees for different 
>> operands?  I guess it could come up, but it'd be a whole lot of extra 
>> complexity that I can't imagine we'd ever support.
> 
> You mean, if `element_size` is passed then you get different guarantees?

No, sorry, I mean different guarantees for the different pointer operands.  In 
principle, we could allow you to say that the memcpy has to be done with 4-byte 
accesses from the source and 2-byte accesses to the destination.  That's 
implementable but a lot of work.

>> If one of the arguments is `volatile`, arguably the minimum access width (if 
>> given) needs to be exact.  If we don't support that right now, it's okay to 
>> make it an error, which is basically you've already done with the `_Atomic 
>> volatile` diagnostic.
> 
> Agreed. `volatile` with size makes a lot of sense, and the IR version of it, 
> once created, ought to not be able to widen accesses. `volatile` without a 
> size specified makes sense too, because you just want a single read and a 
> single write, don't care about tearing.

Right.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79279



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

[PATCH] D79279: Add overloaded versions of builtin mem* functions

[JF Bastien via Phabricator via cfe-commits]

jfb added a comment.

>>> Do you think it'd be useful to have different guarantees for different 
>>> operands?  I guess it could come up, but it'd be a whole lot of extra 
>>> complexity that I can't imagine we'd ever support.
>> 
>> You mean, if `element_size` is passed then you get different guarantees?
> 
> No, sorry, I mean different guarantees for the different pointer operands.  
> In principle, we could allow you to say that the memcpy has to be done with 
> 4-byte accesses from the source and 2-byte accesses to the destination.  
> That's implementable but a lot of work.

Gotcha. Yeah I think it's useful as a niche thing, and if IR supports that for 
say `volatile` then we can honor lopsided `volatile` overloads (instead of 
treating the entire thing as `volatile`). I hadn't really thought about 
lopsided access sizes (since it fell out of `_Atomic`). Maybe it's useful? I 
was just banning unequal sizes before because it seemed like a mistake to copy 
to/from different types. If we wanted to support it, I suppose we could add 
another optional parameter, so I'm OK not doing it now, and adding later if 
useful.

Alright, I'll update the patch as discussed, thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79279



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

[PATCH] D84422: [OpenMP] Fix `present` for exit from `omp target data`

[George Rokos via Phabricator via cfe-commits]

grokos added a comment.

What confuses me about this interpretation of the standard is the inconsistency 
at `data exit`. So if we have an explicit `omp target exit data 
map(present...)` then we should respect the "present" semantics, whereas when 
we have a scoped data exit:

  #pragma omp target data map(present,...)
  {
...
  } // implicit "exit data" here

then "present" should be ignored.

I agree that the paragraph from the standard leaves little room for other 
interpretations, I'd just like to point out that it looks inconsistent - at 
least to me.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84422



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

[PATCH] D84356: [AIX] remove -u from the clang when invoke aix as assembler

[Jason Liu via Phabricator via cfe-commits]

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

Thanks. LGTM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84356



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

[PATCH] D80858: [CUDA][HIP] Support accessing static device variable in host code

[Michael Liao via Phabricator via cfe-commits]

hliao added a comment.

In D80858#2169399 , @yaxunl wrote:

> In D80858#2169295 , @hliao wrote:
>
> > I don't that's proper way to support file-scope static device variables. As 
> > we discuss APIs like cudaMemcpyToSymol, that's a runtime API instead of 
> > driver API. The later needs to specify the module (or code object) id in 
> > addition to a symbol name and won't have the conflict issues. For the 
> > runtime API, all named device variables (static or not) are identified at 
> > the host side by their host stub variables. That stub variable is used to 
> > register the corresponding device variables associated with a module id to 
> > unique identify that variables across all TUs. As long as we look up device 
> > variables using their host stub variable pointers, they are uniquely 
> > identified already. The runtime implementation needs to find the module id 
> > and the variable symbol from the pointer of its host stub variable. It's 
> > not the compiler job to fabricate name uniquely across TUs.
>
>
> The problem is that even though the static variable is registered through 
> `__hipRigisterVariable`, the runtime still relies on looking up symbol name 
> to get the address of the device variable. That's why we need to externalize 
> the static variable.


If so, the runtime should be fixed as the variable name. I remembered I fixed 
the global one so that each one is uniquely identified by module id plus the 
name. For runtime APIs, all host-side references to device variables should 
look through the host stub variables instead of its name. If runtime or API 
doesn't follow that, we should fix them instead of asking the compiler to do 
the favor.


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

https://reviews.llvm.org/D80858



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

[PATCH] D84453: [clang-tidy] Suppress one unittest under ASan.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ created this revision.
NoQ added reviewers: alexfh, gribozavr, dcoughlin, delcypher, yln, kubamracek, 
vsavchenko.
Herald added subscribers: martong, mgehre, Charusso, xazax.hun.

It's failing on macOS under AddressSanitizer for no obvious reason - i.e., no 
report is produced, just the code isn't working correctly. Might possibly be a 
bug in ASan but I want to silence an internal buildbot before a deeper 
investigation. I didn't try platforms other than macOS.

The failure can be reproduced somewhere around 
rG0b2a92224630f6e177d091b8391cfa943764aba5 
 by 
self-hosting. The unittest has to be compiled as ` -DCMAKE_BUILD_TYPE=Release 
-DLLVM_USE_SANITIZER="Address"` in order to reproduce the issue 
(`RelWithDebInfo` works too, but not `Debug`).


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D84453

Files:
  clang-tools-extra/unittests/clang-tidy/ClangTidyDiagnosticConsumerTest.cpp


Index: 
clang-tools-extra/unittests/clang-tidy/ClangTidyDiagnosticConsumerTest.cpp
===
--- clang-tools-extra/unittests/clang-tidy/ClangTidyDiagnosticConsumerTest.cpp
+++ clang-tools-extra/unittests/clang-tidy/ClangTidyDiagnosticConsumerTest.cpp
@@ -21,6 +21,9 @@
   }
 };
 
+// FIXME: This test currently fails under ASan without any actual ASan report.
+// It's as if runCheckOnCode() is not invoked at all. Must be a bug.
+#if !defined(__has_feature) || !__has_feature(address_sanitizer)
 TEST(ClangTidyDiagnosticConsumer, SortsErrors) {
   std::vector Errors;
   runCheckOnCode("int a;", &Errors);
@@ -28,6 +31,7 @@
   EXPECT_EQ("type specifier", Errors[0].Message.Message);
   EXPECT_EQ("variable", Errors[1].Message.Message);
 }
+#endif
 
 } // namespace test
 } // namespace tidy


Index: clang-tools-extra/unittests/clang-tidy/ClangTidyDiagnosticConsumerTest.cpp
===
--- clang-tools-extra/unittests/clang-tidy/ClangTidyDiagnosticConsumerTest.cpp
+++ clang-tools-extra/unittests/clang-tidy/ClangTidyDiagnosticConsumerTest.cpp
@@ -21,6 +21,9 @@
   }
 };
 
+// FIXME: This test currently fails under ASan without any actual ASan report.
+// It's as if runCheckOnCode() is not invoked at all. Must be a bug.
+#if !defined(__has_feature) || !__has_feature(address_sanitizer)
 TEST(ClangTidyDiagnosticConsumer, SortsErrors) {
   std::vector Errors;
   runCheckOnCode("int a;", &Errors);
@@ -28,6 +31,7 @@
   EXPECT_EQ("type specifier", Errors[0].Message.Message);
   EXPECT_EQ("variable", Errors[1].Message.Message);
 }
+#endif
 
 } // namespace test
 } // namespace tidy
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D80858: [CUDA][HIP] Support accessing static device variable in host code

[Michael Liao via Phabricator via cfe-commits]

hliao added a comment.

In D80858#2169534 , @yaxunl wrote:

> Another reason is that we need to support it in rdc mode, where different TU 
> can have static var with the same name.


That's an issue of our current RDC support through LLVM IR instead of native 
code. The name conflicts are introduced as we link all TUs into a single module 
at IR level. The frontend should not be changed to support that.


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

https://reviews.llvm.org/D80858



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

[PATCH] D84414: [RISCV] Support Shadow Call Stack

[Z. Zheng via Phabricator via cfe-commits]

zzheng updated this revision to Diff 280210.
zzheng marked 3 inline comments as done.
zzheng edited the summary of this revision.
zzheng added a comment.

Using 'BLOCKED' now.

clang-formated RISCVFrameLowering.cpp

updated style of test/CodeGen/RISCV/shadowcallstack.ll


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84414

Files:
  clang/lib/Driver/SanitizerArgs.cpp
  clang/lib/Driver/ToolChain.cpp
  clang/test/CodeGen/shadowcallstack-attr.c
  clang/test/Driver/sanitizer-ld.c
  llvm/lib/Target/RISCV/RISCVFrameLowering.cpp
  llvm/test/CodeGen/RISCV/shadowcallstack.ll

Index: llvm/test/CodeGen/RISCV/shadowcallstack.ll
===
--- /dev/null
+++ llvm/test/CodeGen/RISCV/shadowcallstack.ll
@@ -0,0 +1,179 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc -mtriple=riscv32-unknown-elf -mattr=+reserve-x18 -verify-machineinstrs < %s \
+; RUN: | FileCheck %s --check-prefix=RV32
+
+; RUN: llc -mtriple=riscv64-unknown-elf -mattr=+reserve-x18 -verify-machineinstrs < %s \
+; RUN: | FileCheck %s --check-prefix=RV64
+
+define void @f1() shadowcallstack {
+; RV32-LABEL: f1:
+; RV32:   # %bb.0:
+; RV32-NEXT:ret
+; RV32-NOT: x18
+;
+; RV64-LABEL: f1:
+; RV64:   # %bb.0:
+; RV64-NEXT:ret
+; RV64-NOT: x18
+  ret void
+}
+
+declare void @foo()
+
+define void @f2() shadowcallstack {
+; RV32-LABEL: f2:
+; RV32:   # %bb.0:
+; RV32-NEXT:tail foo
+; RV32-NOT: x18
+;
+; RV64-LABEL: f2:
+; RV64:   # %bb.0:
+; RV64-NEXT:tail foo
+; RV64-NOT: x18
+  tail call void @foo()
+  ret void
+}
+
+declare i32 @bar()
+
+define i32 @f3() shadowcallstack {
+; RV32-LABEL: f3:
+; RV32:   # %bb.0:
+; RV32-NEXT:sw ra, 0(s2)
+; RV32-NEXT:addi s2, s2, 4
+; RV32-NEXT:addi sp, sp, -16
+; RV32-NEXT:.cfi_def_cfa_offset 16
+; RV32-NEXT:sw ra, 12(sp)
+; RV32-NEXT:.cfi_offset ra, -4
+; RV32-NEXT:call bar
+; RV32-NEXT:lw ra, 12(sp)
+; RV32-NEXT:addi sp, sp, 16
+; RV32-NEXT:addi s2, s2, -4
+; RV32-NEXT:lw ra, 0(s2)
+; RV32-NEXT:ret
+;
+; RV64-LABEL: f3:
+; RV64:   # %bb.0:
+; RV64-NEXT:sw ra, 0(s2)
+; RV64-NEXT:addi s2, s2, 8
+; RV64-NEXT:addi sp, sp, -16
+; RV64-NEXT:.cfi_def_cfa_offset 16
+; RV64-NEXT:sd ra, 8(sp)
+; RV64-NEXT:.cfi_offset ra, -8
+; RV64-NEXT:call bar
+; RV64-NEXT:ld ra, 8(sp)
+; RV64-NEXT:addi sp, sp, 16
+; RV64-NEXT:addi s2, s2, -8
+; RV64-NEXT:lw ra, 0(s2)
+; RV64-NEXT:ret
+  %res = call i32 @bar()
+  %res1 = add i32 %res, 1
+  ret i32 %res
+}
+
+define i32 @f4() shadowcallstack {
+; RV32-LABEL: f4:
+; RV32:   # %bb.0:
+; RV32-NEXT:sw ra, 0(s2)
+; RV32-NEXT:addi s2, s2, 4
+; RV32-NEXT:addi sp, sp, -16
+; RV32-NEXT:.cfi_def_cfa_offset 16
+; RV32-NEXT:sw ra, 12(sp)
+; RV32-NEXT:sw s0, 8(sp)
+; RV32-NEXT:sw s1, 4(sp)
+; RV32-NEXT:sw s3, 0(sp)
+; RV32-NEXT:.cfi_offset ra, -4
+; RV32-NEXT:.cfi_offset s0, -8
+; RV32-NEXT:.cfi_offset s1, -12
+; RV32-NEXT:.cfi_offset s3, -16
+; RV32-NEXT:call bar
+; RV32-NEXT:mv s3, a0
+; RV32-NEXT:call bar
+; RV32-NEXT:mv s1, a0
+; RV32-NEXT:call bar
+; RV32-NEXT:mv s0, a0
+; RV32-NEXT:call bar
+; RV32-NEXT:add a1, s3, s1
+; RV32-NEXT:add a0, s0, a0
+; RV32-NEXT:add a0, a1, a0
+; RV32-NEXT:lw s3, 0(sp)
+; RV32-NEXT:lw s1, 4(sp)
+; RV32-NEXT:lw s0, 8(sp)
+; RV32-NEXT:lw ra, 12(sp)
+; RV32-NEXT:addi sp, sp, 16
+; RV32-NEXT:addi s2, s2, -4
+; RV32-NEXT:lw ra, 0(s2)
+; RV32-NEXT:ret
+;
+; RV64-LABEL: f4:
+; RV64:   # %bb.0:
+; RV64-NEXT:sw ra, 0(s2)
+; RV64-NEXT:addi s2, s2, 8
+; RV64-NEXT:addi sp, sp, -32
+; RV64-NEXT:.cfi_def_cfa_offset 32
+; RV64-NEXT:sd ra, 24(sp)
+; RV64-NEXT:sd s0, 16(sp)
+; RV64-NEXT:sd s1, 8(sp)
+; RV64-NEXT:sd s3, 0(sp)
+; RV64-NEXT:.cfi_offset ra, -8
+; RV64-NEXT:.cfi_offset s0, -16
+; RV64-NEXT:.cfi_offset s1, -24
+; RV64-NEXT:.cfi_offset s3, -32
+; RV64-NEXT:call bar
+; RV64-NEXT:mv s3, a0
+; RV64-NEXT:call bar
+; RV64-NEXT:mv s1, a0
+; RV64-NEXT:call bar
+; RV64-NEXT:mv s0, a0
+; RV64-NEXT:call bar
+; RV64-NEXT:add a1, s3, s1
+; RV64-NEXT:add a0, s0, a0
+; RV64-NEXT:addw a0, a1, a0
+; RV64-NEXT:ld s3, 0(sp)
+; RV64-NEXT:ld s1, 8(sp)
+; RV64-NEXT:ld s0, 16(sp)
+; RV64-NEXT:ld ra, 24(sp)
+; RV64-NEXT:addi sp, sp, 32
+; RV64-NEXT:addi s2, s2, -8
+; RV64-NEXT:lw ra, 0(s2)
+; RV64-NEXT:ret
+  %res1 = call i32 @bar()
+  %res2 = call i32 @bar()
+  %res3 = call i32 @bar()
+  %res4 = call i32 @bar()
+  %res12 = add i32 %res1, %res2
+  %res34 = add i32 %res3, %res4
+  %res1234 = add i32 %res12, %res34
+  ret i32 %res1234
+}
+
+define i32 @f5() shadowcallstack nounwind {
+; RV32-LABEL: f5:
+; RV32:   # %bb.0:
+; RV32-NEX

[PATCH] D84414: [RISCV] Support Shadow Call Stack

[Z. Zheng via Phabricator via cfe-commits]

zzheng added inline comments.



Comment at: llvm/lib/Target/RISCV/RISCVFrameLowering.cpp:95
+  .addReg(RISCV::X18)
+  .addImm(0);
+}

apazos wrote:
> There are thee things to observe here and other reviewers might have some 
> additional comments:
> 
> - RISC-V does not have a reserved platform register like AAch64. The patch 
> uses one of the RISC-V callee saved registers, x18, which happens to coincide 
> with AArch64's register. It is possible to select another register, and 
> additional checks for the flag combo "-fsanitize=shadow-call-stack 
> -ffixed-" will have to be added.
> 
> - The  return address is saved on both the SCS (whose location is 
> protected/hidden) and also in the regular stack. But the return from a 
> function uses the value saved on SCS. The understanding is that not saving it 
> in the regular stack can impact debugging.
> 
> - The SCS is ascending, while the regular stack, by RISC-V convention, is 
> descending. The SCS is not used for passing parameters between calls like the 
> regular stack, so it seems to be ok. But this can be changed too. AArch64 's 
> SCS is also ascending.
Thanks for the clarification, Ana.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84414



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

[PATCH] D84453: [clang-tidy] Suppress one unittest under ASan.

[Valeriy Savchenko via Phabricator via cfe-commits]

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

LGTM, thanks for taking care of this!


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D84453



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

[PATCH] D84348: WIP: Add complete id-expression support to syntax trees

[Eduardo Caldas via Phabricator via cfe-commits]

eduucaldas updated this revision to Diff 280216.
eduucaldas added a comment.

Update API to new nested-name-specifier grammar rule


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84348

Files:
  clang/include/clang/Tooling/Syntax/Nodes.h
  clang/lib/Tooling/Syntax/BuildTree.cpp
  clang/lib/Tooling/Syntax/Nodes.cpp
  clang/unittests/Tooling/Syntax/TreeTest.cpp

Index: clang/unittests/Tooling/Syntax/TreeTest.cpp
===
--- clang/unittests/Tooling/Syntax/TreeTest.cpp
+++ clang/unittests/Tooling/Syntax/TreeTest.cpp
@@ -988,13 +988,13 @@
 | | | |-NestedNameSpecifier
 | | | | |-GlobalNameSpecifier
 | | | | | `-::
-| | | | |-NamespaceNameSpecifier
+| | | | |-IdentifierNameSpecifier
 | | | | | |-n
 | | | | | `-::
-| | | | |-TypeNameSpecifier
+| | | | |-IdentifierNameSpecifier
 | | | | | |-S
 | | | | | `-::
-| | | | `-TypeWithTemplateNameSpecifier
+| | | | `-SimpleTemplateNameSpecifier
 | | | |   |-template
 | | | |   |-TS
 | | | |   |-<
@@ -1010,13 +1010,13 @@
 | |-UnknownExpression
 | | |-IdExpression
 | | | |-NestedNameSpecifier
-| | | | |-NamespaceNameSpecifier
+| | | | |-IdentifierNameSpecifier
 | | | | | |-n
 | | | | | `-::
-| | | | |-TypeNameSpecifier
+| | | | |-IdentifierNameSpecifier
 | | | | | |-S
 | | | | | `-::
-| | | | `-TypeNameSpecifier
+| | | | `-SimpleTemplateNameSpecifier
 | | | |   |-TS
 | | | |   |-<
 | | | |   |-int
@@ -1031,13 +1031,13 @@
 | |-UnknownExpression
 | | |-IdExpression
 | | | |-NestedNameSpecifier
-| | | | |-TypeNameSpecifier
+| | | | |-SimpleTemplateNameSpecifier
 | | | | | |-TS
 | | | | | |-<
 | | | | | |-int
 | | | | | |->
 | | | | | `-::
-| | | | `-TypeNameSpecifier
+| | | | `-IdentifierNameSpecifier
 | | | |   |-S
 | | | |   `-::
 | | | `-UnqualifiedId
@@ -1052,13 +1052,13 @@
 | |-UnknownExpression
 | | |-IdExpression
 | | | |-NestedNameSpecifier
-| | | | |-TypeNameSpecifier
+| | | | |-SimpleTemplateNameSpecifier
 | | | | | |-TS
 | | | | | |-<
 | | | | | |-int
 | | | | | |->
 | | | | | `-::
-| | | | `-TypeNameSpecifier
+| | | | `-IdentifierNameSpecifier
 | | | |   |-S
 | | | |   `-::
 | | | |-template
@@ -1116,10 +1116,10 @@
   | |-UnknownExpression
   | | |-IdExpression
   | | | |-NestedNameSpecifier
-  | | | | |-TypeNameSpecifier
+  | | | | |-IdentifierNameSpecifier
   | | | | | |-T
   | | | | | `-::
-  | | | | `-TypeWithTemplateNameSpecifier
+  | | | | `-SimpleTemplateNameSpecifier
   | | | |   |-template
   | | | |   |-U
   | | | |   |-<
@@ -1135,7 +1135,7 @@
   | |-UnknownExpression
   | | |-IdExpression
   | | | |-NestedNameSpecifier
-  | | | | |-TypeNameSpecifier
+  | | | | |-IdentifierNameSpecifier
   | | | | | |-T
   | | | | | `-::
   | | | | `-IdentifierNameSpecifier
@@ -1150,7 +1150,7 @@
   | |-UnknownExpression
   | | |-IdExpression
   | | | |-NestedNameSpecifier
-  | | | | `-TypeNameSpecifier
+  | | | | `-IdentifierNameSpecifier
   | | | |   |-T
   | | | |   `-::
   | | | |-template
@@ -1217,7 +1217,7 @@
 | |-UnknownExpression
 | | |-IdExpression
 | | | |-NestedNameSpecifier
-| | | | `-TypeNameSpecifier
+| | | | `-DecltypeNameSpecifier
 | | | |   |-decltype
 | | | |   |-(
 | | | |   |-IdExpression
Index: clang/lib/Tooling/Syntax/Nodes.cpp
===
--- clang/lib/Tooling/Syntax/Nodes.cpp
+++ clang/lib/Tooling/Syntax/Nodes.cpp
@@ -118,16 +118,12 @@
 return OS << "MemberPointer";
   case NodeKind::GlobalNameSpecifier:
 return OS << "GlobalNameSpecifier";
-  case NodeKind::NamespaceNameSpecifier:
-return OS << "NamespaceNameSpecifier";
-  case NodeKind::TypeNameSpecifier:
-return OS << "TypeNameSpecifier";
-  case NodeKind::UnknownNameSpecifier:
-return OS << "UnknownNameSpecifier";
+  case NodeKind::DecltypeNameSpecifier:
+return OS << "DecltypeNameSpecifier";
   case NodeKind::IdentifierNameSpecifier:
 return OS << "IdentifierNameSpecifier";
-  case NodeKind::TypeWithTemplateNameSpecifier:
-return OS << "TypeWithTemplateNameSpecifier";
+  case NodeKind::SimpleTemplateNameSpecifier:
+return OS << "SimpleTemplateNameSpecifier";
   case NodeKind::NestedNameSpecifier:
 return OS << "NestedNameSpecifier";
   }
Index: clang/lib/Tooling/Syntax/BuildTree.cpp
===
--- clang/lib/Tooling/Syntax/BuildTree.cpp
+++ clang/lib/Tooling/Syntax/BuildTree.cpp
@@ -744,23 +744,37 @@
 return true;
   }
 
-  syntax::NameSpecifier *
-  BuildNameSpecifier(NestedNameSpecifier::SpecifierKind K) {
-sw

[PATCH] D83915: [PowerPC] Remove QPX/A2Q BGQ/BGP support

[Hal Finkel via Phabricator via cfe-commits]

hfinkel added a comment.

Thanks for working on this. Please, however, take another pass of the test 
cases (especially those that are not in the PowerPC directory).  Many of those 
should not be deleted, please change triple instead. They're testing general 
functionality.




Comment at: 
llvm/test/Transforms/DeadStoreElimination/combined-partial-overwrites.ll:1
-; RUN: opt -S -dse -enable-dse-partial-store-merging=false < %s | FileCheck %s
-target datalayout = "E-m:e-i64:64-n32:64"

This test should not be deleted. Change the triple.



Comment at: llvm/test/Transforms/EntryExitInstrumenter/mcount.ll:1
-; RUN: opt 
-passes="function(ee-instrument),cgscc(inline),function(post-inline-ee-instrument)"
 -S < %s | FileCheck %s
-

This test should not be deleted. It's testing general functionality. The triple 
should be changed.



Comment at: openmp/runtime/src/kmp_platform.h:71
-#undef KMP_OS_CNK
-#define KMP_OS_CNK 1
-#endif

Was this not used anywhere? Or is there more to delete?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83915



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

[PATCH] D84455: [Concepts] Fix a deserialization crash.

[Haojian Wu via Phabricator via cfe-commits]

hokein created this revision.
hokein added a reviewer: rsmith.
Herald added a subscriber: kristof.beyls.
Herald added a project: clang.

`TemplateTypeParmDecl::hasTypeConstraint` is not a safe guard for
checking `TemplateTypeParmDecl::getTypeConstraint()` result is null.

in somecases (e.g. implicit deduction guide templates synthesized from the
constructor), hasTypeConstraint returns false, and getTypeConstraint
returns a nullptr.

Fix https://bugs.llvm.org/show_bug.cgi?id=46790


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D84455

Files:
  clang/lib/Serialization/ASTReaderDecl.cpp
  clang/test/PCH/cxx2a-constraints-crash.cpp


Index: clang/test/PCH/cxx2a-constraints-crash.cpp
===
--- /dev/null
+++ clang/test/PCH/cxx2a-constraints-crash.cpp
@@ -0,0 +1,29 @@
+// RUN: %clang_cc1 -std=c++2a -emit-pch %s -o %t
+// RUN: %clang_cc1 -std=c++2a -include-pch %t -verify %s
+
+// expected-no-diagnostics
+
+#ifndef HEADER
+#define HEADER
+
+template 
+concept not_same_as = true;
+
+template 
+struct subrange {
+  template  R>
+  subrange(R) requires(Kind == 0);
+
+  template  R>
+  subrange(R) requires(Kind != 0);
+};
+
+template 
+subrange(R) -> subrange<42>;
+
+int main() {
+  int c;
+  subrange s(c);
+}
+
+#endif
\ No newline at end of file
Index: clang/lib/Serialization/ASTReaderDecl.cpp
===
--- clang/lib/Serialization/ASTReaderDecl.cpp
+++ clang/lib/Serialization/ASTReaderDecl.cpp
@@ -2909,9 +2909,11 @@
   return false;
 if (TX->hasTypeConstraint() != TY->hasTypeConstraint())
   return false;
-if (TX->hasTypeConstraint()) {
-  const TypeConstraint *TXTC = TX->getTypeConstraint();
-  const TypeConstraint *TYTC = TY->getTypeConstraint();
+const TypeConstraint *TXTC = TX->getTypeConstraint();
+const TypeConstraint *TYTC = TY->getTypeConstraint();
+if (!TXTC != !TYTC)
+  return false;
+if (TXTC && TYTC) {
   if (TXTC->getNamedConcept() != TYTC->getNamedConcept())
 return false;
   if (TXTC->hasExplicitTemplateArgs() != TYTC->hasExplicitTemplateArgs())


Index: clang/test/PCH/cxx2a-constraints-crash.cpp
===
--- /dev/null
+++ clang/test/PCH/cxx2a-constraints-crash.cpp
@@ -0,0 +1,29 @@
+// RUN: %clang_cc1 -std=c++2a -emit-pch %s -o %t
+// RUN: %clang_cc1 -std=c++2a -include-pch %t -verify %s
+
+// expected-no-diagnostics
+
+#ifndef HEADER
+#define HEADER
+
+template 
+concept not_same_as = true;
+
+template 
+struct subrange {
+  template  R>
+  subrange(R) requires(Kind == 0);
+
+  template  R>
+  subrange(R) requires(Kind != 0);
+};
+
+template 
+subrange(R) -> subrange<42>;
+
+int main() {
+  int c;
+  subrange s(c);
+}
+
+#endif
\ No newline at end of file
Index: clang/lib/Serialization/ASTReaderDecl.cpp
===
--- clang/lib/Serialization/ASTReaderDecl.cpp
+++ clang/lib/Serialization/ASTReaderDecl.cpp
@@ -2909,9 +2909,11 @@
   return false;
 if (TX->hasTypeConstraint() != TY->hasTypeConstraint())
   return false;
-if (TX->hasTypeConstraint()) {
-  const TypeConstraint *TXTC = TX->getTypeConstraint();
-  const TypeConstraint *TYTC = TY->getTypeConstraint();
+const TypeConstraint *TXTC = TX->getTypeConstraint();
+const TypeConstraint *TYTC = TY->getTypeConstraint();
+if (!TXTC != !TYTC)
+  return false;
+if (TXTC && TYTC) {
   if (TXTC->getNamedConcept() != TYTC->getNamedConcept())
 return false;
   if (TXTC->hasExplicitTemplateArgs() != TYTC->hasExplicitTemplateArgs())
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D84414: [RISCV] Support Shadow Call Stack

[Jessica Clarke via Phabricator via cfe-commits]

jrtc27 requested changes to this revision.
jrtc27 added a comment.
This revision now requires changes to proceed.

1. Please use local variables with meaningful names for `RISCV::Xn` rather than 
inlining them everywhere and making it harder at a glance to work out what's 
going on without knowing what the ABI register names are.

2. Please make a `RISCVABI::getSCSPReg` or similar function to avoid 
hard-coding X18 in a bunch of places.

3. I'm not convinced a callee-saved register makes any more sense than anything 
else. Whatever you pick, compatibility only works one way round. If foo uses an 
SCS and calls bar that doesn't, then being callee saved helps you, but if bar 
calls foo then (a) foo will try to dereference SCSP (to store then later load), 
likely faulting, perhaps instead "just" clobbering arbitrary memory (b) if foo 
manages to return back to bar without faulting then bar would expect the 
register to have been saved, but it hasn't, an ABI violation. If you use a 
caller-saved register instead then bar can call foo just fine, but foo can't 
call bar as it'll clobber its SCSP. Reserving any register like this is a 
fundamental ABI incompatibility, so picking x18 because it's callee-saved is 
meaningless, and picking it to avoid duplicating 6 lines of code (or fewer, if 
the existing 6 lines are refactored) isn't a good reason either. It's 
ultimately arbitrary, but it should be at least be based on some kind of valid 
reasoning if such reasoning exists. X18 may or may not be a right answer.

  (One might initially think that this incompatibility is fine if you're 
building an executable with SCSP that calls other libraries without SCSP, but 
that breaks down as soon as callbacks exist, as well as more niche things like 
symbol preemption.)




Comment at: llvm/lib/Target/RISCV/RISCVFrameLowering.cpp:33-37
+  std::vector &CSI = MF.getFrameInfo().getCalleeSavedInfo();
+  if (std::none_of(CSI.begin(), CSI.end(), [](CalleeSavedInfo &CSR) {
+return CSR.getReg() == RISCV::X1;
+  }))
+return;

```
if (find(CSI, RISCV::X1) == CSI.end())
  return;
```
(using `llvm::find` as a convenient wrapper around `std::find`, ie shorthand 
for `std::find(CSI.begin(), CSI.end(), RISCV::X1)`). Though personally I'd 
prefer to see X1 come from `RI.getRARegister()` rather than be hard-coded; 
other functions in this file already hard-code it, but in our CHERI fork we 
need to replace RISCV::X1 with RISCV::C1 everywhere so have changed those. 
Having said that, CHERI renders a shadow call stack unnecessary, so I don't 
particularly care if it's broken there, personally. But I still think it's 
nicer code.



Comment at: llvm/lib/Target/RISCV/RISCVFrameLowering.cpp:54
+  // addi s2, s2, 4
+  BuildMI(MBB, MI, DL, TII->get(RISCV::SW))
+  .addReg(RISCV::X1)

This is wrong for RV64.



Comment at: llvm/lib/Target/RISCV/RISCVFrameLowering.cpp:69-73
+  std::vector &CSI = MF.getFrameInfo().getCalleeSavedInfo();
+  if (std::none_of(CSI.begin(), CSI.end(), [](CalleeSavedInfo &CSR) {
+return CSR.getReg() == RISCV::X1;
+  }))
+return;

As above.



Comment at: llvm/lib/Target/RISCV/RISCVFrameLowering.cpp:94
+  .addImm(-SlotSize);
+  BuildMI(MBB, MI, DL, TII->get(RISCV::LW))
+  .addReg(RISCV::X1, RegState::Define)

Also wrong for RV64.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84414



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

[PATCH] D83731: Add Debug Info Size to Symbol Status

[Yifan Shen via Phabricator via cfe-commits]

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



Comment at: lldb/test/API/tools/lldb-vscode/module/TestVSCode_module.py:55-61
+active_modules = self.vscode.get_active_modules()
+program_module = active_modules[program_basename]
+self.assertEqual(program_basename, program_module['name'])
+self.assertEqual(program, program_module['path'])
+self.assertIn('symbolFilePath', program_module)
+self.assertEqual(symbols_path, program_module['symbolFilePath'])
+self.assertIn('addressRange', program_module)

clayborg wrote:
> Unindent everything after the self.waitUntil()? Otherwise we are not testing 
> the program, symbolFilePath and addressRange on symbols with size.
When unindenting these codes, the dsym test for darwin fails as the symbol 
paths don't match. One is using dysm path, one is using a.out.path.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83731



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

[PATCH] D84414: [RISCV] Support Shadow Call Stack

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added inline comments.



Comment at: llvm/lib/Target/RISCV/RISCVFrameLowering.cpp:33-37
+  std::vector &CSI = MF.getFrameInfo().getCalleeSavedInfo();
+  if (std::none_of(CSI.begin(), CSI.end(), [](CalleeSavedInfo &CSR) {
+return CSR.getReg() == RISCV::X1;
+  }))
+return;

jrtc27 wrote:
> ```
> if (find(CSI, RISCV::X1) == CSI.end())
>   return;
> ```
> (using `llvm::find` as a convenient wrapper around `std::find`, ie shorthand 
> for `std::find(CSI.begin(), CSI.end(), RISCV::X1)`). Though personally I'd 
> prefer to see X1 come from `RI.getRARegister()` rather than be hard-coded; 
> other functions in this file already hard-code it, but in our CHERI fork we 
> need to replace RISCV::X1 with RISCV::C1 everywhere so have changed those. 
> Having said that, CHERI renders a shadow call stack unnecessary, so I don't 
> particularly care if it's broken there, personally. But I still think it's 
> nicer code.
`!llvm::is_contained(CSI, RISCV::X1)`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84414



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

[PATCH] D80858: [CUDA][HIP] Support accessing static device variable in host code

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added a comment.

In D80858#2170311 , @hliao wrote:

> > The problem is that even though the static variable is registered through 
> > `__hipRigisterVariable`, the runtime still relies on looking up symbol name 
> > to get the address of the device variable. That's why we need to 
> > externalize the static variable.
>
> If so, the runtime should be fixed as the variable name. I remembered I fixed 
> the global one so that each one is uniquely identified by module id plus the 
> name. For runtime APIs, all host-side references to device variables should 
> look through the host stub variables instead of its name. If runtime or API 
> doesn't follow that, we should fix them instead of asking the compiler to do 
> the favor.


For runtime APIs, we do reference device variables through host stub variable 
instead of its name. However, how does runtime implements that internally?

In host compilation, clang emits call of `__hipRegisterVariable(shadow_var, 
device_var_name)` in init functions.

runtime builds a map of each shadow var to a device var name. then runtime 
looks up device var name in code object to get the real address of a device var.

Note: `__hipRegisterVariable` does not really associate a shadow var with the 
address of a device var, since in host compilation there is no way to know the 
address of a device var. It only associates a shadow var with the name of a 
device var.

So eventually, runtime still needs to look up the device var symbol in code 
objects. Since ROCm runtime does not look up local symbols, it cannot find the 
static device var in code objects, unless we externalize it.


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

https://reviews.llvm.org/D80858



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