[PATCH] D105909: [clang][CallGraphSection] Add type id metadata to indirect call and targets

[Prabhu Karthikeyan Rajasekaran via Phabricator via cfe-commits]

Prabhuk updated this revision to Diff 558152.
Prabhuk added a comment.

Rebased the patchset and addressed the compilation and test failures


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105909

Files:
  llvm/include/llvm/CodeGen/AsmPrinter.h
  llvm/include/llvm/MC/MCObjectFileInfo.h
  llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
  llvm/lib/MC/MCObjectFileInfo.cpp
  llvm/test/CodeGen/call-graph-section.ll

Index: llvm/test/CodeGen/call-graph-section.ll
===
--- /dev/null
+++ llvm/test/CodeGen/call-graph-section.ll
@@ -0,0 +1,73 @@
+; Tests that we store the type identifiers in .callgraph section of the binary.
+
+; RUN: llc --call-graph-section -filetype=obj -o - < %s | \
+; RUN: llvm-readelf -x .callgraph - | FileCheck %s
+
+target triple = "x86_64-unknown-linux-gnu"
+
+define dso_local void @foo() #0 !type !4 {
+entry:
+  ret void
+}
+
+define dso_local i32 @bar(i8 signext %a) #0 !type !5 {
+entry:
+  %a.addr = alloca i8, align 1
+  store i8 %a, i8* %a.addr, align 1
+  ret i32 0
+}
+
+define dso_local i32* @baz(i8* %a) #0 !type !6 {
+entry:
+  %a.addr = alloca i8*, align 8
+  store i8* %a, i8** %a.addr, align 8
+  ret i32* null
+}
+
+define dso_local i32 @main() #0 !type !7 {
+entry:
+  %retval = alloca i32, align 4
+  %fp_foo = alloca void (...)*, align 8
+  %a = alloca i8, align 1
+  %fp_bar = alloca i32 (i8)*, align 8
+  %fp_baz = alloca i32* (i8*)*, align 8
+  store i32 0, i32* %retval, align 4
+  store void (...)* bitcast (void ()* @foo to void (...)*), void (...)** %fp_foo, align 8
+  %0 = load void (...)*, void (...)** %fp_foo, align 8
+  call void (...) %0() [ "type"(metadata !"_ZTSFvE.generalized") ]
+  store i32 (i8)* @bar, i32 (i8)** %fp_bar, align 8
+  %1 = load i32 (i8)*, i32 (i8)** %fp_bar, align 8
+  %2 = load i8, i8* %a, align 1
+  %call = call i32 %1(i8 signext %2) [ "type"(metadata !"_ZTSFicE.generalized") ]
+  store i32* (i8*)* @baz, i32* (i8*)** %fp_baz, align 8
+  %3 = load i32* (i8*)*, i32* (i8*)** %fp_baz, align 8
+  %call1 = call i32* %3(i8* %a) [ "type"(metadata !"_ZTSFPvS_E.generalized") ]
+  call void @foo() [ "type"(metadata !"_ZTSFvE.generalized") ]
+  %4 = load i8, i8* %a, align 1
+  %call2 = call i32 @bar(i8 signext %4) [ "type"(metadata !"_ZTSFicE.generalized") ]
+  %call3 = call i32* @baz(i8* %a) [ "type"(metadata !"_ZTSFPvS_E.generalized") ]
+  ret i32 0
+}
+
+attributes #0 = { noinline nounwind optnone uwtable "frame-pointer"="all" "min-legal-vector-width"="0" "no-trapping-math"="true" "stack-protector-buffer-size"="8" "target-cpu"="x86-64" "target-features"="+cx8,+fxsr,+mmx,+sse,+sse2,+x87" "tune-cpu"="generic" }
+
+!llvm.module.flags = !{!0, !1, !2}
+!llvm.ident = !{!3}
+
+; Check that the numeric type id (md5 hash) for the below type ids are emitted
+; to the callgraph section.
+
+; CHECK: Hex dump of section '.callgraph':
+
+!0 = !{i32 1, !"wchar_size", i32 4}
+!1 = !{i32 7, !"uwtable", i32 1}
+!2 = !{i32 7, !"frame-pointer", i32 2}
+!3 = !{!"clang version 13.0.0 (g...@github.com:llvm/llvm-project.git 6d35f403b91c2f2c604e23763f699d580370ca96)"}
+; CHECK-DAG: 2444f731 f5eecb3e
+!4 = !{i64 0, !"_ZTSFvE.generalized"}
+; CHECK-DAG: 5486bc59 814b8e30
+!5 = !{i64 0, !"_ZTSFicE.generalized"}
+; CHECK-DAG: 7ade6814 f897fd77
+!6 = !{i64 0, !"_ZTSFPvS_E.generalized"}
+; CHECK-DAG: caaf769a 600968fa
+!7 = !{i64 0, !"_ZTSFiE.generalized"}
Index: llvm/lib/MC/MCObjectFileInfo.cpp
===
--- llvm/lib/MC/MCObjectFileInfo.cpp
+++ llvm/lib/MC/MCObjectFileInfo.cpp
@@ -530,6 +530,8 @@
   EHFrameSection =
   Ctx->getELFSection(".eh_frame", EHSectionType, EHSectionFlags);
 
+  CallGraphSection = Ctx->getELFSection(".callgraph", ELF::SHT_PROGBITS, 0);
+
   StackSizesSection = Ctx->getELFSection(".stack_sizes", ELF::SHT_PROGBITS, 0);
 
   PseudoProbeSection = Ctx->getELFSection(".pseudo_probe", DebugSecType, 0);
@@ -1112,6 +1114,24 @@
   llvm_unreachable("Unknown ObjectFormatType");
 }
 
+MCSection *
+MCObjectFileInfo::getCallGraphSection(const MCSection ) const {
+  if (Ctx->getObjectFileType() != MCContext::IsELF)
+return CallGraphSection;
+
+  const MCSectionELF  = static_cast(TextSec);
+  unsigned Flags = ELF::SHF_LINK_ORDER;
+  StringRef GroupName;
+  if (const MCSymbol *Group = ElfSec.getGroup()) {
+GroupName = Group->getName();
+Flags |= ELF::SHF_GROUP;
+  }
+
+  return Ctx->getELFSection(".callgraph", ELF::SHT_PROGBITS, Flags, 0,
+GroupName, true, ElfSec.getUniqueID(),
+cast(TextSec.getBeginSymbol()));
+}
+
 MCSection *
 MCObjectFileInfo::getStackSizesSection(const MCSection ) const {
   if ((Ctx->getObjectFileType() != MCContext::IsELF) ||
Index: llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
===
--- llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp

[PATCH] D105907: [CallGraphSection] Add call graph section options and documentation

[Prabhu Karthikeyan Rajasekaran via Phabricator via cfe-commits]

Prabhuk updated this revision to Diff 558151.
Prabhuk added a comment.

Rebased the patchset and addressed the compilation failures


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105907

Files:
  llvm/include/llvm/CodeGen/AsmPrinter.h
  llvm/include/llvm/MC/MCObjectFileInfo.h
  llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
  llvm/lib/MC/MCObjectFileInfo.cpp
  llvm/test/CodeGen/call-graph-section.ll

Index: llvm/test/CodeGen/call-graph-section.ll
===
--- /dev/null
+++ llvm/test/CodeGen/call-graph-section.ll
@@ -0,0 +1,73 @@
+; Tests that we store the type identifiers in .callgraph section of the binary.
+
+; RUN: llc --call-graph-section -filetype=obj -o - < %s | \
+; RUN: llvm-readelf -x .callgraph - | FileCheck %s
+
+target triple = "x86_64-unknown-linux-gnu"
+
+define dso_local void @foo() #0 !type !4 {
+entry:
+  ret void
+}
+
+define dso_local i32 @bar(i8 signext %a) #0 !type !5 {
+entry:
+  %a.addr = alloca i8, align 1
+  store i8 %a, i8* %a.addr, align 1
+  ret i32 0
+}
+
+define dso_local i32* @baz(i8* %a) #0 !type !6 {
+entry:
+  %a.addr = alloca i8*, align 8
+  store i8* %a, i8** %a.addr, align 8
+  ret i32* null
+}
+
+define dso_local i32 @main() #0 !type !7 {
+entry:
+  %retval = alloca i32, align 4
+  %fp_foo = alloca void (...)*, align 8
+  %a = alloca i8, align 1
+  %fp_bar = alloca i32 (i8)*, align 8
+  %fp_baz = alloca i32* (i8*)*, align 8
+  store i32 0, i32* %retval, align 4
+  store void (...)* bitcast (void ()* @foo to void (...)*), void (...)** %fp_foo, align 8
+  %0 = load void (...)*, void (...)** %fp_foo, align 8
+  call void (...) %0() [ "type"(metadata !"_ZTSFvE.generalized") ]
+  store i32 (i8)* @bar, i32 (i8)** %fp_bar, align 8
+  %1 = load i32 (i8)*, i32 (i8)** %fp_bar, align 8
+  %2 = load i8, i8* %a, align 1
+  %call = call i32 %1(i8 signext %2) [ "type"(metadata !"_ZTSFicE.generalized") ]
+  store i32* (i8*)* @baz, i32* (i8*)** %fp_baz, align 8
+  %3 = load i32* (i8*)*, i32* (i8*)** %fp_baz, align 8
+  %call1 = call i32* %3(i8* %a) [ "type"(metadata !"_ZTSFPvS_E.generalized") ]
+  call void @foo() [ "type"(metadata !"_ZTSFvE.generalized") ]
+  %4 = load i8, i8* %a, align 1
+  %call2 = call i32 @bar(i8 signext %4) [ "type"(metadata !"_ZTSFicE.generalized") ]
+  %call3 = call i32* @baz(i8* %a) [ "type"(metadata !"_ZTSFPvS_E.generalized") ]
+  ret i32 0
+}
+
+attributes #0 = { noinline nounwind optnone uwtable "frame-pointer"="all" "min-legal-vector-width"="0" "no-trapping-math"="true" "stack-protector-buffer-size"="8" "target-cpu"="x86-64" "target-features"="+cx8,+fxsr,+mmx,+sse,+sse2,+x87" "tune-cpu"="generic" }
+
+!llvm.module.flags = !{!0, !1, !2}
+!llvm.ident = !{!3}
+
+; Check that the numeric type id (md5 hash) for the below type ids are emitted
+; to the callgraph section.
+
+; CHECK: Hex dump of section '.callgraph':
+
+!0 = !{i32 1, !"wchar_size", i32 4}
+!1 = !{i32 7, !"uwtable", i32 1}
+!2 = !{i32 7, !"frame-pointer", i32 2}
+!3 = !{!"clang version 13.0.0 (g...@github.com:llvm/llvm-project.git 6d35f403b91c2f2c604e23763f699d580370ca96)"}
+; CHECK-DAG: 2444f731 f5eecb3e
+!4 = !{i64 0, !"_ZTSFvE.generalized"}
+; CHECK-DAG: 5486bc59 814b8e30
+!5 = !{i64 0, !"_ZTSFicE.generalized"}
+; CHECK-DAG: 7ade6814 f897fd77
+!6 = !{i64 0, !"_ZTSFPvS_E.generalized"}
+; CHECK-DAG: caaf769a 600968fa
+!7 = !{i64 0, !"_ZTSFiE.generalized"}
Index: llvm/lib/MC/MCObjectFileInfo.cpp
===
--- llvm/lib/MC/MCObjectFileInfo.cpp
+++ llvm/lib/MC/MCObjectFileInfo.cpp
@@ -530,6 +530,8 @@
   EHFrameSection =
   Ctx->getELFSection(".eh_frame", EHSectionType, EHSectionFlags);
 
+  CallGraphSection = Ctx->getELFSection(".callgraph", ELF::SHT_PROGBITS, 0);
+
   StackSizesSection = Ctx->getELFSection(".stack_sizes", ELF::SHT_PROGBITS, 0);
 
   PseudoProbeSection = Ctx->getELFSection(".pseudo_probe", DebugSecType, 0);
@@ -1112,6 +1114,24 @@
   llvm_unreachable("Unknown ObjectFormatType");
 }
 
+MCSection *
+MCObjectFileInfo::getCallGraphSection(const MCSection ) const {
+  if (Ctx->getObjectFileType() != MCContext::IsELF)
+return CallGraphSection;
+
+  const MCSectionELF  = static_cast(TextSec);
+  unsigned Flags = ELF::SHF_LINK_ORDER;
+  StringRef GroupName;
+  if (const MCSymbol *Group = ElfSec.getGroup()) {
+GroupName = Group->getName();
+Flags |= ELF::SHF_GROUP;
+  }
+
+  return Ctx->getELFSection(".callgraph", ELF::SHT_PROGBITS, Flags, 0,
+GroupName, true, ElfSec.getUniqueID(),
+cast(TextSec.getBeginSymbol()));
+}
+
 MCSection *
 MCObjectFileInfo::getStackSizesSection(const MCSection ) const {
   if ((Ctx->getObjectFileType() != MCContext::IsELF) ||
Index: llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
===
--- llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
+++ 

[PATCH] D105907: [CallGraphSection] Add call graph section options and documentation

[Prabhu Karthikeyan Rajasekaran via Phabricator via cfe-commits]

Prabhuk commandeered this revision.
Prabhuk added a reviewer: necipfazil.
Prabhuk added a comment.
Herald added a project: All.

https://discourse.llvm.org/t/rfc-computing-storing-and-restoring-conservative-call-graphs-with-llvm/58446/3?u=prabhuk


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105907

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

[PATCH] D105909: [clang][CallGraphSection] Add type id metadata to indirect call and targets

[Prabhu Karthikeyan Rajasekaran via Phabricator via cfe-commits]

Prabhuk commandeered this revision.
Prabhuk added a reviewer: necipfazil.
Prabhuk added a comment.
Herald added a project: All.

https://discourse.llvm.org/t/rfc-computing-storing-and-restoring-conservative-call-graphs-with-llvm/58446/3?u=prabhuk


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105909

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

[PATCH] D138846: MC/DC in LLVM Source-Based Code Coverage: LLVM back-end and compiler-rt

[Mike Hommey via Phabricator via cfe-commits]

glandium added a comment.

There is a check already, but it's not enough: 
https://github.com/llvm/llvm-project/issues/52683


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138846

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

[PATCH] D138846: MC/DC in LLVM Source-Based Code Coverage: LLVM back-end and compiler-rt

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

In D138846#4657246 , @hans wrote:

>> I just saw @glandium's earlier comment:
>>
>>> Code built with older versions of LLVM (e.g. rust) and running with the 
>>> updated runtime crash at startup with this change.
>>
>> This is the exact same issue we encountered. Because there is a profile 
>> format change, it's expected to update both clang and rustc to use the same 
>> version of llvm in order for it to work.
>
> Thanks for figuring this out!
>
> Would it be possible to somehow make profile format mismatches a linker error 
> instead of a hard-to-debug runtime problem? For example could the 
> instrumentation depend on some symbol in the runtime whose name includes a 
> version number?
>
> I think the ASan (and maybe other sanitizer) instrumentations do this.

Te detect incompatibilities, we can either rename the metadata section or add a 
version detector field either to the format header or in individual datum.

Renaming seems infeasible since a lot of places need to be modified (difficult 
to make an atomic update everywhere).

This patch does introduce a new version: `Version11` and llvm-profdata will 
seem to report errors for older versions.
Does the Rust side fail to report a version mismatch error?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138846

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

[PATCH] D123235: [OpenMP] atomic compare fail : Parser & AST support

[Alexey Bataev via Phabricator via cfe-commits]

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

LG


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

https://reviews.llvm.org/D123235

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

[PATCH] D123235: [OpenMP] atomic compare fail : Parser & AST support

[Sunil K via Phabricator via cfe-commits]

koops added inline comments.



Comment at: clang/lib/Basic/CMakeLists.txt:4
   TargetParser
+  FrontendOpenMP
   )

koops wrote:
> ABataev wrote:
> > What requires this new dependency?
> When cmake uses -DBUILD_SHARED_LIBS=1 shared libraries are built instead of 
> archive libraries in lib/ directory. 
> Then, I was getting this error:
> `Error in linking libclangBasic.so.18git because getOpenMPClauseName() is not 
> defined.` 
> 
> While linking the .o files to form libFrontendOpenMP.so, `OMP.cpp.o` is also 
> included which has the function definition of `getOpenMPClauseName()`.  Hence 
> there was a need to have libFrontendOpenMP.so in the link line of 
> libclangBasic.so.18git.
The error `Error in linking libclangBasic.so.18git because 
getOpenMPClauseName() is not defined.` was seen on ppc64le and can be 
reproduced on other platforms using -DBUILD_SHARED_LIBS=1 in the cmake command 
line.


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

https://reviews.llvm.org/D123235

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

[PATCH] D123235: [OpenMP] atomic compare fail : Parser & AST support

[Sunil K via Phabricator via cfe-commits]

koops requested review of this revision.
koops added inline comments.



Comment at: clang/lib/Basic/CMakeLists.txt:4
   TargetParser
+  FrontendOpenMP
   )

ABataev wrote:
> What requires this new dependency?
When cmake uses -DBUILD_SHARED_LIBS=1 shared libraries are built instead of 
archive libraries in lib/ directory. 
Then, I was getting this error:
`Error in linking libclangBasic.so.18git because getOpenMPClauseName() is not 
defined.` 

While linking the .o files to form libFrontendOpenMP.so, `OMP.cpp.o` is also 
included which has the function definition of `getOpenMPClauseName()`.  Hence 
there was a need to have libFrontendOpenMP.so in the link line of 
libclangBasic.so.18git.


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

https://reviews.llvm.org/D123235

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

[PATCH] D123235: [OpenMP] atomic compare fail : Parser & AST support

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.



Comment at: clang/lib/Basic/CMakeLists.txt:4
   TargetParser
+  FrontendOpenMP
   )

What requires this new dependency?


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

https://reviews.llvm.org/D123235

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

[PATCH] D123235: [OpenMP] atomic compare fail : Parser & AST support

[Sunil K via Phabricator via cfe-commits]

koops updated this revision to Diff 558146.
koops added a comment.

Adding libFrontendOpenMP.so as a dependent library when building 
libclangBasic.so. Shared libraries were not built and tested by default, hence 
build and failed when checking on ppc64le.


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

https://reviews.llvm.org/D123235

Files:
  clang/include/clang/AST/OpenMPClause.h
  clang/include/clang/AST/RecursiveASTVisitor.h
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Basic/OpenMPKinds.def
  clang/include/clang/Basic/OpenMPKinds.h
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/OpenMPClause.cpp
  clang/lib/AST/StmtProfile.cpp
  clang/lib/Basic/CMakeLists.txt
  clang/lib/Basic/OpenMPKinds.cpp
  clang/lib/CodeGen/CGStmtOpenMP.cpp
  clang/lib/Parse/ParseOpenMP.cpp
  clang/lib/Sema/SemaOpenMP.cpp
  clang/lib/Sema/TreeTransform.h
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTWriter.cpp
  clang/test/OpenMP/atomic_ast_print.cpp
  clang/test/OpenMP/atomic_messages.cpp
  clang/tools/libclang/CIndex.cpp
  flang/lib/Semantics/check-omp-structure.cpp
  llvm/include/llvm/Frontend/OpenMP/OMP.td

Index: llvm/include/llvm/Frontend/OpenMP/OMP.td
===
--- llvm/include/llvm/Frontend/OpenMP/OMP.td
+++ llvm/include/llvm/Frontend/OpenMP/OMP.td
@@ -209,6 +209,7 @@
 def OMPC_Update : Clause<"update"> { let clangClass = "OMPUpdateClause"; }
 def OMPC_Capture : Clause<"capture"> { let clangClass = "OMPCaptureClause"; }
 def OMPC_Compare : Clause<"compare"> { let clangClass = "OMPCompareClause"; }
+def OMPC_Fail : Clause<"fail"> { let clangClass = "OMPFailClause"; }
 def OMPC_SeqCst : Clause<"seq_cst"> { let clangClass = "OMPSeqCstClause"; }
 def OMPC_AcqRel : Clause<"acq_rel"> { let clangClass = "OMPAcqRelClause"; }
 def OMPC_Acquire : Clause<"acquire"> { let clangClass = "OMPAcquireClause"; }
@@ -637,7 +638,8 @@
 VersionedClause,
 VersionedClause,
 VersionedClause,
-VersionedClause
+VersionedClause,
+VersionedClause
   ];
 }
 def OMP_Target : Directive<"target"> {
Index: flang/lib/Semantics/check-omp-structure.cpp
===
--- flang/lib/Semantics/check-omp-structure.cpp
+++ flang/lib/Semantics/check-omp-structure.cpp
@@ -2228,6 +2228,7 @@
 CHECK_SIMPLE_CLAUSE(Doacross, OMPC_doacross)
 CHECK_SIMPLE_CLAUSE(OmpxAttribute, OMPC_ompx_attribute)
 CHECK_SIMPLE_CLAUSE(OmpxBare, OMPC_ompx_bare)
+CHECK_SIMPLE_CLAUSE(Fail, OMPC_fail)
 
 CHECK_REQ_SCALAR_INT_CLAUSE(Grainsize, OMPC_grainsize)
 CHECK_REQ_SCALAR_INT_CLAUSE(NumTasks, OMPC_num_tasks)
Index: clang/tools/libclang/CIndex.cpp
===
--- clang/tools/libclang/CIndex.cpp
+++ clang/tools/libclang/CIndex.cpp
@@ -2402,6 +2402,8 @@
 
 void OMPClauseEnqueue::VisitOMPCompareClause(const OMPCompareClause *) {}
 
+void OMPClauseEnqueue::VisitOMPFailClause(const OMPFailClause *) {}
+
 void OMPClauseEnqueue::VisitOMPSeqCstClause(const OMPSeqCstClause *) {}
 
 void OMPClauseEnqueue::VisitOMPAcqRelClause(const OMPAcqRelClause *) {}
Index: clang/test/OpenMP/atomic_messages.cpp
===
--- clang/test/OpenMP/atomic_messages.cpp
+++ clang/test/OpenMP/atomic_messages.cpp
@@ -958,6 +958,24 @@
 // expected-error@+1 {{directive '#pragma omp atomic' cannot contain more than one 'capture' clause}}
 #pragma omp atomic compare compare capture capture
   { v = a; if (a > b) a = b; }
+// expected-error@+1 {{expected 'compare' clause with the 'fail' modifier}}
+#pragma omp atomic fail(seq_cst)
+  if(v == a) { v = a; }
+// expected-error@+1 {{expected '(' after 'fail'}}
+#pragma omp atomic compare fail
+  if(v < a) { v = a; }
+// expected-error@+1 {{expected a memory order clause}}
+#pragma omp atomic compare fail(capture)
+  if(v < a) { v = a; }
+ // expected-error@+2 {{expected ')'}}
+ // expected-note@+1 {{to match this '('}}
+#pragma omp atomic compare fail(seq_cst | acquire)
+  if(v < a) { v = a; }
+// expected-error@+1 {{directive '#pragma omp atomic' cannot contain more than one 'fail' clause}}
+#pragma omp atomic compare fail(relaxed) fail(seq_cst)
+  if(v < a) { v = a; }
+
+
 #endif
   // expected-note@+1 {{in instantiation of function template specialization 'mixed' requested here}}
   return mixed();
Index: clang/test/OpenMP/atomic_ast_print.cpp
===
--- clang/test/OpenMP/atomic_ast_print.cpp
+++ clang/test/OpenMP/atomic_ast_print.cpp
@@ -226,6 +226,12 @@
   { v = a; if (a < b) { a = b; } }
 #pragma omp atomic compare capture hint(6)
   { v = a == b; if (v) a = c; }
+#pragma omp atomic compare fail(acquire)
+  { if (a < c) { a = c; } }
+#pragma omp atomic compare fail(relaxed)
+  { if (a < c) { a = c; } }
+#pragma omp atomic compare fail(seq_cst)
+  { if (a < c) { a = c; } }
 #endif
   return T();
 }

[PATCH] D138846: MC/DC in LLVM Source-Based Code Coverage: LLVM back-end and compiler-rt

[Hans Wennborg via Phabricator via cfe-commits]

hans added a comment.

> I just saw @glandium's earlier comment:
>
>> Code built with older versions of LLVM (e.g. rust) and running with the 
>> updated runtime crash at startup with this change.
>
> This is the exact same issue we encountered. Because there is a profile 
> format change, it's expected to update both clang and rustc to use the same 
> version of llvm in order for it to work.

Thanks for figuring this out!

Would it be possible to somehow make profile format mismatches a linker error 
instead of a hard-to-debug runtime problem? For example could the 
instrumentation depend on some symbol in the runtime whose name includes a 
version number?

I think the ASan (and maybe other sanitizer) instrumentations do this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138846

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

[PATCH] D150083: [clang-format] ObjCPropertyAttributeOrder to sort ObjC property attributes

[Owen Pan via Phabricator via cfe-commits]

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

LGTM except the final nits.




Comment at: clang/lib/Format/ObjCPropertyAttributeOrderFixer.cpp:48
+const FormatToken *BeginTok, const FormatToken *EndTok) const {
+  // If there are zero or one tokens, nothing to do.
+  if (BeginTok == EndTok || BeginTok->Next == EndTok)





Comment at: clang/lib/Format/ObjCPropertyAttributeOrderFixer.cpp:55
+  for (auto Tok = BeginTok; Tok != EndTok; Tok = Tok->Next) {
+if (Tok->is(tok::comma)) {
+  // Ignore the comma separators.





Comment at: clang/lib/Format/ObjCPropertyAttributeOrderFixer.cpp:71
+// BinaryOperator or Identifier)
+if (Tok->Next->is(tok::equal)) {
+  Tok = Tok->Next;





Comment at: clang/lib/Format/ObjCPropertyAttributeOrderFixer.cpp:73
+  Tok = Tok->Next;
+  if (Tok->Next->isNot(tok::identifier)) {
+// If we hit any other kind of token, just bail. It's unusual/illegal.





Comment at: clang/lib/Format/ObjCPropertyAttributeOrderFixer.cpp:82-84
+  // There's no need to sort unless there's more than one thing.
+  if (PropertyAttributes.size() < 2)
+return;

Delete or assert instead.



Comment at: clang/lib/Format/ObjCPropertyAttributeOrderFixer.cpp:103
+  // If there are no removals or shuffling, then don't suggest any fixup.
+  if (Indices.size() == PropertyAttributes.size() && llvm::is_sorted(Indices))
+return;

You can assert the equality of the sizes before the `if` if you like.



Comment at: clang/lib/Format/ObjCPropertyAttributeOrderFixer.cpp:136
+tooling::Replacements , const FormatToken *Tok) const {
+  // Expect `property` to be the very next token or else just bail early.
+  const FormatToken *const PropertyTok = Tok->Next;





Comment at: clang/lib/Format/ObjCPropertyAttributeOrderFixer.cpp:165
+  for (AnnotatedLine *Line : AnnotatedLines) {
+if (!Line->Affected || Line->Type != LT_ObjCProperty)
+  continue;





Comment at: clang/lib/Format/ObjCPropertyAttributeOrderFixer.cpp:175
+for (const auto *Tok = First; Tok != Last; Tok = Tok->Next) {
+  // Skip until the `@` of a `@property` declaration.
+  if (Tok->isNot(TT_ObjCProperty))





Comment at: clang/lib/Format/ObjCPropertyAttributeOrderFixer.cpp:97
+  }
+
+  // Create a "remapping index" on how to reorder the attributes.

jaredgrubb wrote:
> owenpan wrote:
> > Can we assert `PropertyAttributes.size() > 1` here?
> We shouldn't _assert_, as it is valid to have just one. But I can add an 
> early-return for that though. (I'll also early-return if it's zero, which is 
> also legal, eg `@property () int x`)
Now we can.  



Comment at: clang/lib/Format/ObjCPropertyAttributeOrderFixer.cpp:184
+  for (AnnotatedLine *Line : AnnotatedLines) {
+if (!Line->Affected || Line->InPPDirective)
+  continue;

jaredgrubb wrote:
> owenpan wrote:
> > jaredgrubb wrote:
> > > owenpan wrote:
> > > > Why not `InPPDirective`?
> > > I copy-pasted this from `LeftRightQualifierAlignmentFixer::analyze`, 
> > > which I used as a template since I'm still getting used to the codebase. 
> > > I wasn't sure whether this was important, so I left it in. But I don't 
> > > think I have a good reason. 
> > > 
> > > I've added a new test case `SortsInPPDirective` that spot-checks some 
> > > macro definition examples (which did fail unless this 
> > > `Line->InPPDirective` check was removed, as expected.).
> > What about `Line->Type != LT_ObjCProperty` I suggested?
> Ah, I missed that suggestion. 
> 
> I don't have enough understanding of clang-format to say whether that would 
> be correct, but I'll trust you :) 
> 
> When I add this check it doesn't break unit tests or cause any 
> change-in-behavior on my ObjC test repo (~10K .h/.m files). So, it -seems- 
> like this is a valid check to add, so I'll do it!
> When I add this check it doesn't break unit tests or cause any 
> change-in-behavior on my ObjC test repo (~10K .h/.m files).

If it does, then we probably have a bug in setting `LT_ObjCProperty`.  


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

https://reviews.llvm.org/D150083

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

[PATCH] D155688: [PATCH] [llvm] [InstCombine] Canonicalise ADD+GEP

[Dave Green via Phabricator via cfe-commits]

dmgreen added a comment.

I hadn't realized this came from someone at Arm. The performance results I had 
were overall roughly flat, with some improvements and regressions. I think 
there were still some people working through some fixes for some of the 
knock-on effects but with those nothing large would stick out in what I saw.

I would expect Loop Strength Reduction (maybe with CGP) to be able to optimize 
the addressing modes back to something that is optimal for the loop if it can. 
It's not always super reliable though. Might there be something going wrong in 
that pass?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155688

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

[PATCH] D155688: [PATCH] [llvm] [InstCombine] Canonicalise ADD+GEP

[Craig Topper via Phabricator via cfe-commits]

craig.topper added a comment.

After this patch, I'm seeing a lot of `invariant.gep` created by LICM. For 
example, in `LBM_performStreamCollide` in 470.lbm there are 65 of them. On 
RISC-V, these all get created in registers outside the loop and get spilled. Is 
ARM seeing anything like this or do you have more addressing modes that allow 
CodeGenPrepare to bring these back into the loop?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155688

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

[PATCH] D138846: MC/DC in LLVM Source-Based Code Coverage: LLVM back-end and compiler-rt

[Mike Hommey via Phabricator via cfe-commits]

glandium added a comment.

FWIW, it's the first time for as long as I remember that mixing LLVM versions 
causes a runtime crash (that looks like a null deref).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138846

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

[PATCH] D151730: [RISCV] Support target attribute for function

[Craig Topper via Phabricator via cfe-commits]

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

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151730

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

[PATCH] D138846: MC/DC in LLVM Source-Based Code Coverage: LLVM back-end and compiler-rt

[Zequan Wu via Phabricator via cfe-commits]

zequanwu added a comment.

In D138846#4657195 , @zequanwu wrote:

> In D138846#4657194 , @alanphipps 
> wrote:
>
>> In D138846#4657193 , @zequanwu 
>> wrote:
>>
>>> In D138846#4657175 , @hans wrote:
>>>
 We're seeing crashes in `initializeValueProfRuntimeRecord` that bisects to 
 this commit. I think Zequan is investigating: 
 https://bugs.chromium.org/p/chromium/issues/detail?id=1503919
>>>
>>> It turns out to be that our rust code with old version of rustc without 
>>> this change, so mixture of raw profile versions are used, causing segment 
>>> fault.
>>
>> Thank you for the update! Is this also the case of the issue @glandium 
>> reports above as well?  I think both issues manifested as a ValueProf issue.
>
> I don't know, but I'd suggest to check if all sources compiled with profile 
> runtime are using the llvm version that contains this change. Otherwise, 
> mixed raw profiles versions will cause unexpected behaviour.

I just saw @glandium's earlier comment:

> Code built with older versions of LLVM (e.g. rust) and running with the 
> updated runtime crash at startup with this change.

This is the exact same issue we encountered. Because there is a profile format 
change, it's expected to update both clang and rustc to use the same version of 
llvm in order for it to work.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138846

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

[PATCH] D138846: MC/DC in LLVM Source-Based Code Coverage: LLVM back-end and compiler-rt

[Zequan Wu via Phabricator via cfe-commits]

zequanwu added a comment.

In D138846#4657194 , @alanphipps 
wrote:

> In D138846#4657193 , @zequanwu 
> wrote:
>
>> In D138846#4657175 , @hans wrote:
>>
>>> We're seeing crashes in `initializeValueProfRuntimeRecord` that bisects to 
>>> this commit. I think Zequan is investigating: 
>>> https://bugs.chromium.org/p/chromium/issues/detail?id=1503919
>>
>> It turns out to be that our rust code with old version of rustc without this 
>> change, so mixture of raw profile versions are used, causing segment fault.
>
> Thank you for the update! Is this also the case of the issue @glandium 
> reports above as well?  I think both issues manifested as a ValueProf issue.

I don't know, but I'd suggest to check if all sources compiled with profile 
runtime are using the llvm version that contains this change. Otherwise, mixed 
raw profiles versions will cause unexpected behaviour.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138846

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

[PATCH] D138846: MC/DC in LLVM Source-Based Code Coverage: LLVM back-end and compiler-rt

[Alan Phipps via Phabricator via cfe-commits]

alanphipps added a comment.

In D138846#4657193 , @zequanwu wrote:

> In D138846#4657175 , @hans wrote:
>
>> We're seeing crashes in `initializeValueProfRuntimeRecord` that bisects to 
>> this commit. I think Zequan is investigating: 
>> https://bugs.chromium.org/p/chromium/issues/detail?id=1503919
>
> It turns out to be that our rust code with old version of rustc without this 
> change, so mixture of raw profile versions are used, causing segment fault.

Thank you for the update! Is this also the case of the issue @glandium reports 
above as well?  I think both issues manifested as a ValueProf issue.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138846

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

[PATCH] D138846: MC/DC in LLVM Source-Based Code Coverage: LLVM back-end and compiler-rt

[Zequan Wu via Phabricator via cfe-commits]

zequanwu added a comment.

In D138846#4657175 , @hans wrote:

> We're seeing crashes in `initializeValueProfRuntimeRecord` that bisects to 
> this commit. I think Zequan is investigating: 
> https://bugs.chromium.org/p/chromium/issues/detail?id=1503919

It turns out to be that our rust code with old version of rustc without this 
change, so mixture of raw profile versions are used, causing segment fault.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138846

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

[PATCH] D138846: MC/DC in LLVM Source-Based Code Coverage: LLVM back-end and compiler-rt

[Alan Phipps via Phabricator via cfe-commits]

alanphipps added a comment.

In D138846#4657152 , @glandium wrote:

> In D138846#4656607 , @glandium 
> wrote:
>
>> Code built with older versions of LLVM (e.g. rust) and running with the 
>> updated runtime crash at startup with this change.
>
> FWIW, neither followups fixed this issue. The crash happens in 
> __llvm_profile_instrument_target.

Hmm, then it's different from the repro given earlier.  Can you provide a test 
case?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138846

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

[PATCH] D138846: MC/DC in LLVM Source-Based Code Coverage: LLVM back-end and compiler-rt

[Hans Wennborg via Phabricator via cfe-commits]

hans added a comment.

We're seeing crashes in `initializeValueProfRuntimeRecord` that bisects to this 
commit. I think Zequan is investigating: 
https://bugs.chromium.org/p/chromium/issues/detail?id=1503919


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138846

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

[PATCH] D138846: MC/DC in LLVM Source-Based Code Coverage: LLVM back-end and compiler-rt

[Mike Hommey via Phabricator via cfe-commits]

glandium added a comment.

In D138846#4656607 , @glandium wrote:

> Code built with older versions of LLVM (e.g. rust) and running with the 
> updated runtime crash at startup with this change.

FWIW, neither followups fixed this issue. The crash happens in 
__llvm_profile_instrument_target.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138846

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

[PATCH] D149573: [Clang][C++23] Implement core language changes from P1467R9 extended floating-point types and standard names

[Joshua Cranmer via Phabricator via cfe-commits]

jcranmer-intel added a comment.

So while trying to review this patch, I've discovered there's an annoying 
incompatibility between C and C++ here, in that C and C++ specify different 
rules on how to choose between `_Float64`, `double`, and `long double` if all 
are `binary64` (C says `_Float64` > `long double` > `double`, C++ says `long 
double` > `_Float64` > `double`), and I don't have enough knowledge of clang 
internal implementation to know if the changes being done can capture the 
different semantics there. (It's also not entirely clear to me that the 
incompatibility between C and C++ was intentional in the first place; it looks 
like the paper author intentionally chose the ordering for C++ and argued for 
the change to C, but the CFP working group soundly rejected the change, and 
personally I agree with the CFP decision here over C++).




Comment at: clang/lib/AST/ASTContext.cpp:136-138
+// another floating point type.
+constexpr std::array, 5>
+CXX23FloatingPointConversionRankMap = {

I don't like having a large table here of results. It just doesn't scale; if 
you take into account all of the putative types that might be supported, you've 
got 3 basic types + 4 interchange formats + 3 decimal types + 1 IEEE extended 
type + bfloat + 3 IBM hex floats, and that's just things already supported by 
LLVM or that I know someone's working on.

I think after special casing float/double/long double, you can handle all the 
other types by simply using a mixture of `fltSemantics::isRepresentableBy` and 
a subrank preference list.

In the event that support for `_Float32` and `_Float64` are added, this table 
will no longer be target-independent. We have a few targets where `double` is 
binary32 and several targets where `long double` is either binary32 or binary64 
(and others where it's neither). I think it's better to start writing this 
method in a way that can handle this mapping be target-dependent.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149573

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

[PATCH] D149573: [Clang][C++23] Implement core language changes from P1467R9 extended floating-point types and standard names

[Zahira Ammarguellat via Phabricator via cfe-commits]

zahiraam added a comment.

Just starting to look at this. Don't we need a RN for this?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149573

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

[PATCH] D123235: [OpenMP] atomic compare fail : Parser & AST support

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

FWIW, this appears to have broken some build bots: 
https://lab.llvm.org/buildbot/#/builders/121/builds/36635 -- can you revert and 
investigate (or fix-forward quickly)?


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

https://reviews.llvm.org/D123235

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

[PATCH] D151730: [RISCV] Support target attribute for function

[Piyou Chen via Phabricator via cfe-commits]

BeMg updated this revision to Diff 558134.
BeMg added a comment.

1. reuse the find result
2. replace for loop with llvm::copy_if
3. use explicit std::vector declaration instead of auto


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151730

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Basic/Targets/RISCV.cpp
  clang/lib/Basic/Targets/RISCV.h
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/CodeGen/RISCV/riscv-func-attr-target-err.c
  clang/test/CodeGen/RISCV/riscv-func-attr-target.c

Index: clang/test/CodeGen/RISCV/riscv-func-attr-target.c
===
--- /dev/null
+++ clang/test/CodeGen/RISCV/riscv-func-attr-target.c
@@ -0,0 +1,46 @@
+// REQUIRES: riscv-registered-target
+// RUN: %clang_cc1 -triple riscv64 -target-feature +zifencei -target-feature +m \
+// RUN:  -target-feature +a -target-feature +save-restore \
+// RUN:  -emit-llvm %s -o - | FileCheck %s
+
+// CHECK-LABEL: define dso_local void @testDefault
+// CHECK-SAME: () #0 {
+void testDefault() {}
+// CHECK-LABEL: define dso_local void @testMultiAttrStr
+// CHECK-SAME: () #1 {
+__attribute__((target("cpu=rocket-rv64;tune=generic-rv64;arch=+v"))) void
+testMultiAttrStr() {}
+// CHECK-LABEL: define dso_local void @testSingleExtension
+// CHECK-SAME: () #2 {
+__attribute__((target("arch=+zbb"))) void testSingleExtension() {}
+// CHECK-LABEL: define dso_local void @testMultiExtension
+// CHECK-SAME: () #3 {
+__attribute__((target("arch=+zbb,+v,+zicond"))) void testMultiExtension() {}
+// CHECK-LABEL: define dso_local void @testFullArch
+// CHECK-SAME: () #4 {
+__attribute__((target("arch=rv64gc_zbb"))) void testFullArch() {}
+// CHECK-LABEL: define dso_local void @testFullArchButSmallThanCmdArch
+// CHECK-SAME: () #5 {
+__attribute__((target("arch=rv64im"))) void testFullArchButSmallThanCmdArch() {}
+// CHECK-LABEL: define dso_local void @testAttrArchAndAttrCpu
+// CHECK-SAME: () #6 {
+__attribute__((target("cpu=sifive-u54;arch=+zbb"))) void
+testAttrArchAndAttrCpu() {}
+// CHECK-LABEL: define dso_local void @testAttrFullArchAndAttrCpu
+// CHECK-SAME: () #7 {
+__attribute__((target("cpu=sifive-u54;arch=rv64im"))) void
+testAttrFullArchAndAttrCpu() {}
+// CHECK-LABEL: define dso_local void @testAttrCpuOnly
+// CHECK-SAME: () #8 {
+__attribute__((target("cpu=sifive-u54"))) void testAttrCpuOnly() {}
+
+//.
+// CHECK: attributes #0 = { {{.*}}"target-features"="+64bit,+a,+m,+save-restore,+zifencei" }
+// CHECK: attributes #1 = { {{.*}}"target-cpu"="rocket-rv64" "target-features"="+64bit,+a,+d,+f,+m,+save-restore,+v,+zicsr,+zifencei,+zve32f,+zve32x,+zve64d,+zve64f,+zve64x,+zvl128b,+zvl32b,+zvl64b" "tune-cpu"="generic-rv64" }
+// CHECK: attributes #2 = { {{.*}}"target-features"="+64bit,+a,+m,+save-restore,+zbb,+zifencei" }
+// CHECK: attributes #3 = { {{.*}}"target-features"="+64bit,+a,+d,+experimental-zicond,+f,+m,+save-restore,+v,+zbb,+zicsr,+zifencei,+zve32f,+zve32x,+zve64d,+zve64f,+zve64x,+zvl128b,+zvl32b,+zvl64b" }
+// CHECK: attributes #4 = { {{.*}}"target-features"="+64bit,+a,+c,+d,+f,+m,+save-restore,+zbb,+zicsr,+zifencei" }
+// CHECK: attributes #5 = { {{.*}}"target-features"="+64bit,+m,+save-restore" }
+// CHECK: attributes #6 = { {{.*}}"target-cpu"="sifive-u54" "target-features"="+64bit,+a,+m,+save-restore,+zbb,+zifencei" }
+// CHECK: attributes #7 = { {{.*}}"target-cpu"="sifive-u54" "target-features"="+64bit,+m,+save-restore" }
+// CHECK: attributes #8 = { {{.*}}"target-cpu"="sifive-u54" "target-features"="+64bit,+a,+c,+d,+f,+m,+save-restore,+zicsr,+zifencei" }
Index: clang/test/CodeGen/RISCV/riscv-func-attr-target-err.c
===
--- /dev/null
+++ clang/test/CodeGen/RISCV/riscv-func-attr-target-err.c
@@ -0,0 +1,10 @@
+// REQUIRES: riscv-registered-target
+// RUN: not %clang_cc1 -triple riscv64 -target-feature +zifencei -target-feature +m -target-feature +a \
+// RUN:  -emit-llvm %s 2>&1 | FileCheck %s
+
+// CHECK: error: duplicate 'arch=' in the 'target' attribute string;
+__attribute__((target("arch=rv64gc;arch=rv64gc_zbb"))) void testMultiArchSelectLast() {}
+// CHECK: error: duplicate 'cpu=' in the 'target' attribute string;
+__attribute__((target("cpu=sifive-u74;cpu=sifive-u54"))) void testMultiCpuSelectLast() {}
+// CHECK: error: duplicate 'tune=' in the 'target' attribute string;
+__attribute__((target("tune=sifive-u74;tune=sifive-u54"))) void testMultiTuneSelectLast() {}
Index: clang/lib/Sema/SemaDeclAttr.cpp
===
--- clang/lib/Sema/SemaDeclAttr.cpp
+++ clang/lib/Sema/SemaDeclAttr.cpp
@@ -3450,6 +3450,11 @@
 return Diag(LiteralLoc, diag::warn_unsupported_target_attribute)
<< Unknown << Tune << ParsedAttrs.Tune << Target;
 
+  if (Context.getTargetInfo().getTriple().isRISCV() &&
+  ParsedAttrs.Duplicate != "")
+return Diag(LiteralLoc, 

[PATCH] D70401: [RISCV] CodeGen of RVE and ilp32e/lp64e ABIs

[Wang Pengcheng via Phabricator via cfe-commits]

wangpc added a comment.

In D70401#4657101 , @jrtc27 wrote:

> In D70401#4657098 , @jrtc27 wrote:
>
>> GCC only ever defines __riscv_32e
>
> Hm, seems the comments about __riscv_32e were from months ago, ignore them if 
> they aren't correct or have become outdated...

FYI: https://github.com/riscv-non-isa/riscv-c-api-doc/pull/52


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70401

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

[PATCH] D70401: [RISCV] CodeGen of RVE and ilp32e/lp64e ABIs

[Jessica Clarke via Phabricator via cfe-commits]

jrtc27 added a comment.

In D70401#4657098 , @jrtc27 wrote:

> GCC only ever defines __riscv_32e

Hm, seems the comments about __riscv_32e were from months ago, ignore them if 
they aren't correct or have become outdated...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70401

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

[PATCH] D70401: [RISCV] CodeGen of RVE and ilp32e/lp64e ABIs

[Jessica Clarke via Phabricator via cfe-commits]

jrtc27 added a comment.

GCC only ever defines __riscv_32e




Comment at: clang/lib/Basic/Targets/RISCV.cpp:210
+if (Is64Bit)
+  Builder.defineMacro("__riscv_64e");
+else

Ugh, these don't align with the normal pattern. __riscv_e already exists in the 
spec, can we just leave __riscv_32e as deprecated for RV32E and not introduce 
the old-style __riscv_64e?



Comment at: clang/lib/Basic/Targets/RISCV.h:139
 if (Name == "ilp32" || Name == "ilp32f" || Name == "ilp32d") {
   ABI = Name;
   return true;

Does it matter we don't undo the effects of the RVE ABI here?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70401

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

[PATCH] D70401: [RISCV] CodeGen of RVE and ilp32e/lp64e ABIs

[Wang Pengcheng via Phabricator via cfe-commits]

wangpc added a comment.

@craig.topper Thanks!
@asb Hi Alex, I'd like to get another approval from you. Are there any more 
concerns?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70401

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

[PATCH] D151730: [RISCV] Support target attribute for function

[Craig Topper via Phabricator via cfe-commits]

craig.topper added inline comments.



Comment at: clang/lib/Basic/Targets/RISCV.cpp:229
+collectNonISAExtFeature(const std::vector , int XLen) 
{
+  auto I = llvm::find(FeaturesVec, "__RISCV_TargetAttrNeedOverride");
+  auto FeatureNeedOveride = std::vector(FeaturesVec.begin(), I);

Do we need to call find again? We already did it in the caller, can we pass 
that information somehow?



Comment at: clang/lib/Basic/Targets/RISCV.cpp:230
+  auto I = llvm::find(FeaturesVec, "__RISCV_TargetAttrNeedOverride");
+  auto FeatureNeedOveride = std::vector(FeaturesVec.begin(), I);
+  auto ParseResult =

`std::vector FeaturesNeedOverride(FeaturesVec.begin(), I);`



Comment at: clang/lib/Basic/Targets/RISCV.cpp:243
+
+  for (auto Feat : FeatureNeedOveride) {
+if (!llvm::is_contained(ImpliedFeatures, Feat))

`llvm::copy_if(FeatureNeedOveride, std::back_inserter(NonISAExtFeatureVec), 
[&](const std::string ) { return !llvm::is_contained(ImpliedFeatures, 
Feat); });`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151730

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

[PATCH] D151730: [RISCV] Support target attribute for function

[Piyou Chen via Phabricator via cfe-commits]

BeMg added a comment.

ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151730

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

[PATCH] D135171: FreeBSD: enable __float128 on x86 and powerpc64le

[Alexander Richardson via Phabricator via cfe-commits]

arichardson added a comment.

In D135171#4657080 , @brad wrote:

> You can close this.

The submitted patch 
https://github.com/llvm/llvm-project/commit/23c47eba879769a29772c999be2991201c2fe399
 was not the same since it omitted ppc64. So I guess this should remain open


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135171

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

[PATCH] D153701: [Clang] Implement P2718R0 "Lifetime extension in range-based for loops"

[Yurong via Phabricator via cfe-commits]

yronglin added a comment.

In D153701#4656920 , @Endill wrote:

> @yronglin We are sorry it takes so much time to get feedback. Richard and 
> Hubert have little bandwidth for reviews. Others, including me, don't feel 
> qualified to provide good feedback.

@Endill Thanks for your reply, let's wait for them have time. You are clang 
experts, please feel free to comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153701

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

[PATCH] D86855: Convert __m64 intrinsics to unconditionally use SSE2 instead of MMX instructions.

[Phoebe Wang via Phabricator via cfe-commits]

pengfei added a comment.

Reverse ping. Any progress or plan for this patch?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86855

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

[PATCH] D135171: FreeBSD: enable __float128 on x86 and powerpc64le

[Brad Smith via Phabricator via cfe-commits]

brad added a comment.

You can close this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135171

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

[PATCH] D150083: [clang-format] ObjCPropertyAttributeOrder to sort ObjC property attributes

[Jared Grubb via Phabricator via cfe-commits]

jaredgrubb updated this revision to Diff 558132.
jaredgrubb marked 3 inline comments as done.
jaredgrubb added a comment.

Update to address all the review comments.


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

https://reviews.llvm.org/D150083

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/CMakeLists.txt
  clang/lib/Format/Format.cpp
  clang/lib/Format/ObjCPropertyAttributeOrderFixer.cpp
  clang/lib/Format/ObjCPropertyAttributeOrderFixer.h
  clang/unittests/Format/CMakeLists.txt
  clang/unittests/Format/ObjCPropertyAttributeOrderFixerTest.cpp

Index: clang/unittests/Format/ObjCPropertyAttributeOrderFixerTest.cpp
===
--- /dev/null
+++ clang/unittests/Format/ObjCPropertyAttributeOrderFixerTest.cpp
@@ -0,0 +1,423 @@
+//===- unittest/Format/ObjCPropertyAttributeOrderFixerTest.cpp - unit tests
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "../lib/Format/ObjCPropertyAttributeOrderFixer.h"
+#include "FormatTestBase.h"
+#include "TestLexer.h"
+
+#define DEBUG_TYPE "format-objc-property-attribute-order-fixer-test"
+
+namespace clang {
+namespace format {
+namespace test {
+namespace {
+
+#define CHECK_PARSE(TEXT, FIELD, VALUE)\
+  EXPECT_NE(VALUE, Style.FIELD) << "Initial value already the same!";  \
+  EXPECT_EQ(0, parseConfiguration(TEXT, ).value());  \
+  EXPECT_EQ(VALUE, Style.FIELD) << "Unexpected value after parsing!"
+
+#define FAIL_PARSE(TEXT, FIELD, VALUE) \
+  EXPECT_NE(0, parseConfiguration(TEXT, ).value());  \
+  EXPECT_EQ(VALUE, Style.FIELD) << "Unexpected value after parsing!"
+
+class ObjCPropertyAttributeOrderFixerTest : public FormatTestBase {
+protected:
+  TokenList annotate(llvm::StringRef Code,
+ const FormatStyle  = getLLVMStyle()) {
+return TestLexer(Allocator, Buffers, Style).annotate(Code);
+  }
+
+  llvm::SpecificBumpPtrAllocator Allocator;
+  std::vector> Buffers;
+};
+
+TEST_F(ObjCPropertyAttributeOrderFixerTest, ParsesStyleOption) {
+  FormatStyle Style = {};
+  Style.Language = FormatStyle::LK_ObjC;
+
+  CHECK_PARSE("ObjCPropertyAttributeOrder: [class]", ObjCPropertyAttributeOrder,
+  std::vector({"class"}));
+
+  CHECK_PARSE("ObjCPropertyAttributeOrder: ["
+  "class, direct, atomic, nonatomic, "
+  "assign, retain, strong, copy, weak, unsafe_unretained, "
+  "readonly, readwrite, getter, setter, "
+  "nullable, nonnull, null_resettable, null_unspecified"
+  "]",
+  ObjCPropertyAttributeOrder,
+  std::vector({
+  "class",
+  "direct",
+  "atomic",
+  "nonatomic",
+  "assign",
+  "retain",
+  "strong",
+  "copy",
+  "weak",
+  "unsafe_unretained",
+  "readonly",
+  "readwrite",
+  "getter",
+  "setter",
+  "nullable",
+  "nonnull",
+  "null_resettable",
+  "null_unspecified",
+  }));
+}
+
+TEST_F(ObjCPropertyAttributeOrderFixerTest, SortsSpecifiedAttributes) {
+  FormatStyle Style = getLLVMStyle();
+  Style.Language = FormatStyle::LK_ObjC;
+  Style.ObjCPropertyAttributeOrder = {"a", "b", "c"};
+
+  // Zero: nothing to do, but is legal.
+  verifyFormat("@property() int p;", Style);
+
+  // One: shouldn't move.
+  verifyFormat("@property(a) int p;", Style);
+  verifyFormat("@property(b) int p;", Style);
+  verifyFormat("@property(c) int p;", Style);
+
+  // Two in correct order already: no change.
+  verifyFormat("@property(a, b) int p;", Style);
+  verifyFormat("@property(a, c) int p;", Style);
+  verifyFormat("@property(b, c) int p;", Style);
+
+  // Three in correct order already: no change.
+  verifyFormat("@property(a, b, c) int p;", Style);
+
+  // Two wrong order.
+  verifyFormat("@property(a, b) int p;", "@property(b, a) int p;", Style);
+  verifyFormat("@property(a, c) int p;", "@property(c, a) int p;", Style);
+  verifyFormat("@property(b, c) int p;", "@property(c, b) int p;", Style);
+
+  // Three wrong order.
+  verifyFormat("@property(a, b, c) int p;", "@property(b, a, c) int p;", Style);
+  verifyFormat("@property(a, b, c) int p;", "@property(c, b, a) int p;", Style);
+
+  // Check that properties preceded by @optional/@required work.
+  verifyFormat("@optional\n"
+   "@property(a, b) int p;",
+  

[PATCH] D150083: [clang-format] ObjCPropertyAttributeOrder to sort ObjC property attributes

[Jared Grubb via Phabricator via cfe-commits]

jaredgrubb marked 7 inline comments as done.
jaredgrubb added inline comments.



Comment at: clang/lib/Format/ObjCPropertyAttributeOrderFixer.cpp:113
+
+  // Deduplicate the attributes.
+  Indices.erase(std::unique(Indices.begin(), Indices.end(),

owenpan wrote:
> jaredgrubb wrote:
> > owenpan wrote:
> > > Is it valid in Objective-C to have duplicate attributes?
> > It's silly, but clang doesn't seem to care. I tried this, so duplicate was 
> > ok, but contradiction was flagged:
> > ```
> > @property (strong, nonatomic, nonatomic) X *X;   // duplicate, but no error
> > @property (strong, nonatomic, weak) Y *y;   // error: Property attributes 
> > 'strong' and 'weak' are mutually exclusive
> > ```
> > 
> > I wasn't sure whether to do this, but went that way since that's what "sort 
> > include files" did. However, it seems like an odd corner case so I'd be ok 
> > removing this uniquing if you prefer.
> We should keep the duplicates if clang accepts them. What happens to e.g. 
> `@property(x=a, y=b, x=a, y=c)`?
Looks like clang does not reject duplicates like that either, even though I 
can't imagine that it results in anything sane:
```
@property (getter=Foo, getter=Bar) int x;
```

I'll remove the de-dup code as you request -- it'll make the pass simpler and 
marginally faster :)

For dup-cases where the values differ (eg, `y=b, y=c`), the pass will just 
stable-sort them. This is the simplest thing to do and IMO is good-enough as I 
think dup's are questionable anyway.

I've added some test cases for this (and removed the original de-dup test 
cases).

I'll add a couple unit tests to confirm this and include your example.



Comment at: clang/lib/Format/ObjCPropertyAttributeOrderFixer.cpp:184
+  for (AnnotatedLine *Line : AnnotatedLines) {
+if (!Line->Affected || Line->InPPDirective)
+  continue;

owenpan wrote:
> jaredgrubb wrote:
> > owenpan wrote:
> > > Why not `InPPDirective`?
> > I copy-pasted this from `LeftRightQualifierAlignmentFixer::analyze`, which 
> > I used as a template since I'm still getting used to the codebase. I wasn't 
> > sure whether this was important, so I left it in. But I don't think I have 
> > a good reason. 
> > 
> > I've added a new test case `SortsInPPDirective` that spot-checks some macro 
> > definition examples (which did fail unless this `Line->InPPDirective` check 
> > was removed, as expected.).
> What about `Line->Type != LT_ObjCProperty` I suggested?
Ah, I missed that suggestion. 

I don't have enough understanding of clang-format to say whether that would be 
correct, but I'll trust you :) 

When I add this check it doesn't break unit tests or cause any 
change-in-behavior on my ObjC test repo (~10K .h/.m files). So, it -seems- like 
this is a valid check to add, so I'll do it!



Comment at: clang/lib/Format/ObjCPropertyAttributeOrderFixer.cpp:190
+  continue;
+
+const auto *Last = Line->Last;

owenpan wrote:
> jaredgrubb wrote:
> > owenpan wrote:
> > > Must `@property` be the first non-comment tokens of an unwrapped line in 
> > > Objective-C? And at most one `@property` per line?
> > I can't think of any token that should precede a `@property`. Aka, I don't 
> > think there are any `__attribute__` that can fill that slot. 
> > 
> > You could have `@optional` or `@required` floating around between 
> > property/method declarations. I've added a test-case for a `@property` that 
> > is preceded by these tokens and proved that the reordering is handled 
> > correctly.
> > 
> > As for multiple properties in one line, I've never seen a codebase with 
> > that. clang-format does split them already.
> I asked the questions because if yes, we could move on to the next line 
> before hitting the last token of the current line.
In testing, it seems that some other pass (not sure which) will split the line 
before this new pass sees it. So if I can rely on that (I think I can?), then 
we can early-break like you propose.

I've added a unit test to cover this scenario.


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

https://reviews.llvm.org/D150083

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

[PATCH] D70401: [RISCV] CodeGen of RVE and ilp32e/lp64e ABIs

[Craig Topper via Phabricator via cfe-commits]

craig.topper accepted this revision.
craig.topper added a comment.

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70401

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

[PATCH] D150083: [clang-format] ObjCPropertyAttributeOrder to sort ObjC property attributes

[Jared Grubb via Phabricator via cfe-commits]

jaredgrubb added a comment.

In D150083#4656832 , @owenpan wrote:

> Thank you for your patience!

I appreciate the help :) I'm excited to get this in!

> In D150083#4655528 , @owenpan wrote:
>
>> See also D153228 .
>
> Would this patch have a similar performance issue?

Reading through the issue, I don't think so. As I understand that issue, the 
scale-problem is due to how the passes interact with one another as a whole 
(since that setting creates a Pass for _each_ qualifier). This ObjC pass uses 
only ONE pass to handle the ordering of all property attributes. I 
intentionally did not copy that part of the Qualifier pass because it didn't 
seem to actually help my use-case, which is much narrower (primarily that I 
only have to operate on lines that look like properties, not in arbitrary 
places like function args and variable declarations, etc.).

Please correct me if I missed something about that patch that could apply.


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

https://reviews.llvm.org/D150083

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

[PATCH] D70401: [RISCV] CodeGen of RVE and ilp32e/lp64e ABIs

[Koute via Phabricator via cfe-commits]

koute added a comment.

Sorry for the comment spam, but could we please get this merged in finally? (:

To people who hold the decision making power as to whether this is merged: are 
there still any blockers left, considering the consensus was to merge it? 
What's the hold up? Is there anything I can do to help?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70401

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

[PATCH] D149573: [Clang][C++23] Implement core language changes from P1467R9 extended floating-point types and standard names

[Tom Honermann via Phabricator via cfe-commits]

tahonermann added a comment.

Apologies once again for the delayed response. I reviewed some today and will 
resume reviewing on Monday.

In addition to the inline suggestions:

`clang/docs/ReleaseNotes.rst` will need to be updated to reflect that the core 
language changes for P1467R9 have been implemented for `std::float16_t` and 
`std::bfloat16_t`.

Given the confusing array of 16-bit floating-point types, how about modifying 
`clang/include/clang/AST/BuiltinTypes.def` to be more explicit regarding which 
is which?

  // 'half' in OpenCL, '__fp16' in ARM NEON.
  FLOATING_TYPE(Half, HalfTy)
  ...
  // '_Float16', 'std::float16_t'
  FLOATING_TYPE(Float16, HalfTy)
  
  // '__bf16', 'std::bfloat16_t'
  FLOATING_TYPE(BFloat16, BFloat16Ty)

Hmm, having pasted the above, I now noticed that the `Half` and `Float16` types 
share the `HalfTy` singleton. Unless I'm mistaken, the changes being made will 
cause divergent behavior. Do these now need to be split?




Comment at: clang/include/clang/AST/ASTContext.h:56
+// Conversion ranks introduced in C++23 6.8.6p2 [conv.rank]
+enum FloatingRankCompareResult {
+  FRCR_Unordered,

Naming suggestion: `FloatConvRankCompareResult`.



Comment at: clang/include/clang/AST/ASTContext.h:1119
+  CanQualType BFloat16Ty; // [C++23 6.8.3p5][basic.extended.fp]
+  CanQualType Float16Ty; // C11 extension ISO/IEC TS 18661-3 and [C++23 
6.8.3p5][basic.extended.fp]
   CanQualType VoidPtrTy, NullPtrTy;

I think the comment should reference http://eel.is/c++draft/basic.extended.fp#1 
for `std::float16_t`. p5 is for `std::bfloat16_t`.



Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:8931
 >;
+def err_cxx23_invalid_implicit_floating_point_cast : Error<"floating point 
cast results in loss of precision">;
 def err_typecheck_expect_scalar_operand : Error<

The diagnostic text here looks more like the text of a warning. Since this is 
an error, I think it makes more sense to model the text on other similar errors 
and use "narrowing" or "implicit cast" terminology. For examples, see 
`err_spaceship_argument_narrowing` and `err_impcast_complex_scalar`

 Additionally, it would be helpful to include the relevant types in the message.

Finally, line length should be kept to 80 characters or less per 
https://llvm.org/docs/CodingStandards.html#source-code-width.



Comment at: 
clang/include/clang/StaticAnalyzer/Core/PathSensitive/SMTConv.h:779-788
+FloatingRankCompareResult order = Ctx.getFloatingTypeOrder(LTy, RTy);
+if ((order == FRCR_Greater) || (order == FRCR_Equal_Greater_Subrank)) {
   RHS = (*doCast)(Solver, RHS, LTy, LBitWidth, RTy, RBitWidth);
   RTy = LTy;
-} else if (order == 0) {
+} else if ((order == FRCR_Equal) || (order == FRCR_Equal_Lesser_Subrank)) {
   LHS = (*doCast)(Solver, LHS, RTy, RBitWidth, LTy, LBitWidth);
   LTy = RTy;

This looks like a pre-existing bug since, for standard floating-point types, 
narrowing implicit conversions are allowed. I think the intent was to add a 
cast on which ever side is needed, but the existing code instead adds a cast on 
the RHS when the LHS has a higher rank, adds a cast on the LHS when the LHS and 
RHS have the same rank, and wanders into UB when the RHS has a higher rank.

The incorrect comparison goes back to when the code was introduced in commit 
08f943c5630d8ee31d6a93227171d2f05db59e62.

The introduction of unordered conversion ranks suggests additional changes are 
needed here, but it isn't clear to me what the right changes are. I added a 
suggested edit that adds an arbitrary cast (which probably suffices for the 
purposes of static analysis). Alternatively, an assert could be added for the 
unordered and equal cases.



Comment at: clang/lib/Sema/SemaChecking.cpp:10451
   return ICE->getCastKind() == CK_FloatingCast &&
- S.Context.getFloatingTypeOrder(From, To) < 0;
+ S.Context.getFloatingTypeOrder(From, To) == FRCR_Lesser;
 }

codemzs wrote:
> tahonermann wrote:
> > I'm not sure this is right. If I understand correctly, the C++23 extended 
> > FP types don't participate in argument promotions. Perhaps they should be 
> > excluded here.
> Rules for 
> [promotion](https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2022/p1467r9.html#promotion)
>  are unchanged as per the proposal. This is just using the new enumeration to 
> represent a smaller conversion rank. 
I think this still produces an incorrect result in some cases though. According 
to http://eel.is/c++draft/conv.fpprom, the only floating-point promotion is 
from `float` to `double`.

Ah, I think the prior code was incorrect, but non-symptomatic because it is 
only currently used in one place (in `CheckPrintfHandler::checkFormatExpr()`) 
and that code only cares about the integer cases. I would prefer that we fix 
this rather than extend the issue 

[PATCH] D123235: [OpenMP] atomic compare fail : Parser & AST support

[Alexey Bataev via Phabricator via cfe-commits]

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

LG


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

https://reviews.llvm.org/D123235

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

[PATCH] D123235: [OpenMP] atomic compare fail : Parser & AST support

[Sunil K via Phabricator via cfe-commits]

koops updated this revision to Diff 558120.
koops added a comment.

Moving the declaration and definition of checkFailClauseParameter to 
include/clang/Basic/OpenMPKinds.h & lib/Basic/OpenMPKinds.cpp respectively.
2 other minor changes suggested by Alexey.


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

https://reviews.llvm.org/D123235

Files:
  clang/include/clang/AST/OpenMPClause.h
  clang/include/clang/AST/RecursiveASTVisitor.h
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Basic/OpenMPKinds.def
  clang/include/clang/Basic/OpenMPKinds.h
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/OpenMPClause.cpp
  clang/lib/AST/StmtProfile.cpp
  clang/lib/Basic/OpenMPKinds.cpp
  clang/lib/CodeGen/CGStmtOpenMP.cpp
  clang/lib/Parse/ParseOpenMP.cpp
  clang/lib/Sema/SemaOpenMP.cpp
  clang/lib/Sema/TreeTransform.h
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTWriter.cpp
  clang/test/OpenMP/atomic_ast_print.cpp
  clang/test/OpenMP/atomic_messages.cpp
  clang/tools/libclang/CIndex.cpp
  flang/lib/Semantics/check-omp-structure.cpp
  llvm/include/llvm/Frontend/OpenMP/OMP.td

Index: llvm/include/llvm/Frontend/OpenMP/OMP.td
===
--- llvm/include/llvm/Frontend/OpenMP/OMP.td
+++ llvm/include/llvm/Frontend/OpenMP/OMP.td
@@ -209,6 +209,7 @@
 def OMPC_Update : Clause<"update"> { let clangClass = "OMPUpdateClause"; }
 def OMPC_Capture : Clause<"capture"> { let clangClass = "OMPCaptureClause"; }
 def OMPC_Compare : Clause<"compare"> { let clangClass = "OMPCompareClause"; }
+def OMPC_Fail : Clause<"fail"> { let clangClass = "OMPFailClause"; }
 def OMPC_SeqCst : Clause<"seq_cst"> { let clangClass = "OMPSeqCstClause"; }
 def OMPC_AcqRel : Clause<"acq_rel"> { let clangClass = "OMPAcqRelClause"; }
 def OMPC_Acquire : Clause<"acquire"> { let clangClass = "OMPAcquireClause"; }
@@ -637,7 +638,8 @@
 VersionedClause,
 VersionedClause,
 VersionedClause,
-VersionedClause
+VersionedClause,
+VersionedClause
   ];
 }
 def OMP_Target : Directive<"target"> {
Index: flang/lib/Semantics/check-omp-structure.cpp
===
--- flang/lib/Semantics/check-omp-structure.cpp
+++ flang/lib/Semantics/check-omp-structure.cpp
@@ -2228,6 +2228,7 @@
 CHECK_SIMPLE_CLAUSE(Doacross, OMPC_doacross)
 CHECK_SIMPLE_CLAUSE(OmpxAttribute, OMPC_ompx_attribute)
 CHECK_SIMPLE_CLAUSE(OmpxBare, OMPC_ompx_bare)
+CHECK_SIMPLE_CLAUSE(Fail, OMPC_fail)
 
 CHECK_REQ_SCALAR_INT_CLAUSE(Grainsize, OMPC_grainsize)
 CHECK_REQ_SCALAR_INT_CLAUSE(NumTasks, OMPC_num_tasks)
Index: clang/tools/libclang/CIndex.cpp
===
--- clang/tools/libclang/CIndex.cpp
+++ clang/tools/libclang/CIndex.cpp
@@ -2402,6 +2402,8 @@
 
 void OMPClauseEnqueue::VisitOMPCompareClause(const OMPCompareClause *) {}
 
+void OMPClauseEnqueue::VisitOMPFailClause(const OMPFailClause *) {}
+
 void OMPClauseEnqueue::VisitOMPSeqCstClause(const OMPSeqCstClause *) {}
 
 void OMPClauseEnqueue::VisitOMPAcqRelClause(const OMPAcqRelClause *) {}
Index: clang/test/OpenMP/atomic_messages.cpp
===
--- clang/test/OpenMP/atomic_messages.cpp
+++ clang/test/OpenMP/atomic_messages.cpp
@@ -958,6 +958,24 @@
 // expected-error@+1 {{directive '#pragma omp atomic' cannot contain more than one 'capture' clause}}
 #pragma omp atomic compare compare capture capture
   { v = a; if (a > b) a = b; }
+// expected-error@+1 {{expected 'compare' clause with the 'fail' modifier}}
+#pragma omp atomic fail(seq_cst)
+  if(v == a) { v = a; }
+// expected-error@+1 {{expected '(' after 'fail'}}
+#pragma omp atomic compare fail
+  if(v < a) { v = a; }
+// expected-error@+1 {{expected a memory order clause}}
+#pragma omp atomic compare fail(capture)
+  if(v < a) { v = a; }
+ // expected-error@+2 {{expected ')'}}
+ // expected-note@+1 {{to match this '('}}
+#pragma omp atomic compare fail(seq_cst | acquire)
+  if(v < a) { v = a; }
+// expected-error@+1 {{directive '#pragma omp atomic' cannot contain more than one 'fail' clause}}
+#pragma omp atomic compare fail(relaxed) fail(seq_cst)
+  if(v < a) { v = a; }
+
+
 #endif
   // expected-note@+1 {{in instantiation of function template specialization 'mixed' requested here}}
   return mixed();
Index: clang/test/OpenMP/atomic_ast_print.cpp
===
--- clang/test/OpenMP/atomic_ast_print.cpp
+++ clang/test/OpenMP/atomic_ast_print.cpp
@@ -226,6 +226,12 @@
   { v = a; if (a < b) { a = b; } }
 #pragma omp atomic compare capture hint(6)
   { v = a == b; if (v) a = c; }
+#pragma omp atomic compare fail(acquire)
+  { if (a < c) { a = c; } }
+#pragma omp atomic compare fail(relaxed)
+  { if (a < c) { a = c; } }
+#pragma omp atomic compare fail(seq_cst)
+  { if (a < c) { a = c; } }
 #endif
   return T();
 }
@@ -1099,6 +1105,12 @@
   { v = 

[PATCH] D154396: [clang] Add support for SerenityOS

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added inline comments.



Comment at: clang/lib/Driver/ToolChains/Serenity.cpp:25
+
+static bool getPIE(const ArgList , const ToolChain ) {
+  if (Args.hasArg(options::OPT_static, options::OPT_shared,

I've simplified `Gnu.cpp` a bit for handling different link modes. 
ae623d16d50c9f12de7ae7ac1aa11c9d6857e081



Comment at: clang/lib/Driver/ToolChains/Serenity.cpp:51
+
+  if (IsPIE || IsStaticPIE)
+CmdArgs.push_back("-pie");

See ae623d16d50c9f12de7ae7ac1aa11c9d6857e081


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154396

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

[PATCH] D123235: [OpenMP] atomic compare fail : Parser & AST support

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.



Comment at: clang/include/clang/AST/OpenMPClause.h:2516
 
+extern bool checkFailParameter(OpenMPClauseKind FailParameter);
+/// This represents 'fail' clause in the '#pragma omp atomic'

No way for extern functions. Declare in include/clang/Basic/OpenMPKinds.h



Comment at: clang/include/clang/AST/OpenMPClause.h:2544-2545
+
+if (!checkFailParameter(FailParameter))
+  llvm_unreachable("Invalid fail clause parameter");
+  }





Comment at: clang/lib/Sema/SemaOpenMP.cpp:17707-17714
+bool clang::checkFailParameter(OpenMPClauseKind FailParameter) {
+  if (FailParameter == llvm::omp::OMPC_acquire ||
+  FailParameter == llvm::omp::OMPC_relaxed ||
+  FailParameter == llvm::omp::OMPC_seq_cst)
+return true;
+
+  return false;

Move it to lib/Basic/OpenMPKinds.cpp



Comment at: clang/lib/Sema/SemaOpenMP.cpp:17721
+
+  if (!clang::checkFailParameter(Parameter)) {
+Diag(KindLoc, diag::err_omp_atomic_fail_wrong_or_no_clauses);




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

https://reviews.llvm.org/D123235

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

[PATCH] D150930: [Driver] Accept and ignore -fno-lifetime-dse argument

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

In D150930#4656909 , @brunodf wrote:

> Hi,
>
> I found this review request and I just want to comment that I find it strange 
> that it was rejected.
>
> @MaskRay I understand that using a compile_commands.json configured for gcc 
> with clang-based tools is not a supported use case, but still the clang 
> driver was explicitly designed to emulate gcc: "The clang tool is the 
> compiler driver and front-end, which is designed to be a drop-in replacement 
> for the gcc command" and "The 'clang' driver is designed to work as closely 
> to GCC as possible to maximize portability." are quotes from 
> https://clang.llvm.org/get_started.html
> In that regard, `clang_ignored_gcc_optimization_f_Group` is logical and it 
> includes many options that you cite like `-falign-jumps=`, `-falign-loops=`, 
> `-fmerge-constants`, `-fschedule-insns`, etc.
> Sure, more projects support clang directly now, but I was not aware there is 
> a change in this policy, or that there is a "stop" on adding more options (in 
> that case, it would be consistent that the documentation is adapted to say 
> that clang is only drop-in compatible with some historic version of gcc).

Clang does emulate a lot of options, but this does not mean we will add every 
option GCC supports.
I think this sentence in my previous reply applies:

> For a useful option, we consider implementing it, but I think the bar is 
> higher than appeasing such unsupported use cases

We want that the project and users learn to remove unsupported options.

Nowadays, `clang_ignored_gcc_optimization_f_Group` should probably not be used 
anymore.

> In my view, the main objection to "accept and ignore" a GCC option is when 
> the option provides some guarantees that clang/LLVM cannot uphold. For 
> example, ignoring `-fno-strict-aliasing` would be dangerous if you actually 
> carry out type-based aliasing optimizations, because programs that compile 
> with it likely contain violations of the strict aliasing rules. It seems that 
> `-fno-lifetime-dse` similarly intends to allow violating a language rule. I'm 
> not aware if clang/LLVM contains optimization that exploit this language rule 
> (since the option appears in the context of the LLVM code base itself, and 
> because compiling this code base with clang itself is well tested in many 
> configurations, I suspect not), but if it does (now or in the future), 
> ignoring this option is dangerous.

I think it's possible that LLVM in the future make take advantage of the 
similar idea behind GCC's `-flifetime-dse`, but the semantics may or may not be 
exactly the same.
And we may choose to implement `-fno-lifetime-dse` or not.

MemorySanitizer's error checking is a bit like `-flifetime-dse` 
(https://github.com/llvm/llvm-project/issues/24952).
If we implement `-flifetime-dse`, we may need to think about its interaction 
with MemorySanitizer, which brings more complexity.
Right now, the justification for adding an ignored option is not significant 
enough.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150930

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

[PATCH] D123235: [OpenMP] atomic compare fail : Parser & AST support

[Sunil K via Phabricator via cfe-commits]

koops updated this revision to Diff 558112.
koops added a comment.

1. Added a check for the fail parameter before the instance of OMPFailClause is 
created in ActOnOpenMPFailClause. An error in fail parameter like `#pragma omp 
atomic compare fail(capture)` was creating a wrong instance of OMPFailClause 
with OMPC_unknown as parameter.
2. In `g++ 12.3` and spec, `fail(acq_rel)` and `fail(release)`  are not 
permitted. Removed them from being supported as parameters to the `fail` clause.
3. Checking the parameters to `fail` clause into a single routine 
`clang::checkFailParameter`.


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

https://reviews.llvm.org/D123235

Files:
  clang/include/clang/AST/OpenMPClause.h
  clang/include/clang/AST/RecursiveASTVisitor.h
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Basic/OpenMPKinds.def
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/OpenMPClause.cpp
  clang/lib/AST/StmtProfile.cpp
  clang/lib/Basic/OpenMPKinds.cpp
  clang/lib/CodeGen/CGStmtOpenMP.cpp
  clang/lib/Parse/ParseOpenMP.cpp
  clang/lib/Sema/SemaOpenMP.cpp
  clang/lib/Sema/TreeTransform.h
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTWriter.cpp
  clang/test/OpenMP/atomic_ast_print.cpp
  clang/test/OpenMP/atomic_messages.cpp
  clang/tools/libclang/CIndex.cpp
  flang/lib/Semantics/check-omp-structure.cpp
  llvm/include/llvm/Frontend/OpenMP/OMP.td

Index: llvm/include/llvm/Frontend/OpenMP/OMP.td
===
--- llvm/include/llvm/Frontend/OpenMP/OMP.td
+++ llvm/include/llvm/Frontend/OpenMP/OMP.td
@@ -209,6 +209,7 @@
 def OMPC_Update : Clause<"update"> { let clangClass = "OMPUpdateClause"; }
 def OMPC_Capture : Clause<"capture"> { let clangClass = "OMPCaptureClause"; }
 def OMPC_Compare : Clause<"compare"> { let clangClass = "OMPCompareClause"; }
+def OMPC_Fail : Clause<"fail"> { let clangClass = "OMPFailClause"; }
 def OMPC_SeqCst : Clause<"seq_cst"> { let clangClass = "OMPSeqCstClause"; }
 def OMPC_AcqRel : Clause<"acq_rel"> { let clangClass = "OMPAcqRelClause"; }
 def OMPC_Acquire : Clause<"acquire"> { let clangClass = "OMPAcquireClause"; }
@@ -637,7 +638,8 @@
 VersionedClause,
 VersionedClause,
 VersionedClause,
-VersionedClause
+VersionedClause,
+VersionedClause
   ];
 }
 def OMP_Target : Directive<"target"> {
Index: flang/lib/Semantics/check-omp-structure.cpp
===
--- flang/lib/Semantics/check-omp-structure.cpp
+++ flang/lib/Semantics/check-omp-structure.cpp
@@ -2228,6 +2228,7 @@
 CHECK_SIMPLE_CLAUSE(Doacross, OMPC_doacross)
 CHECK_SIMPLE_CLAUSE(OmpxAttribute, OMPC_ompx_attribute)
 CHECK_SIMPLE_CLAUSE(OmpxBare, OMPC_ompx_bare)
+CHECK_SIMPLE_CLAUSE(Fail, OMPC_fail)
 
 CHECK_REQ_SCALAR_INT_CLAUSE(Grainsize, OMPC_grainsize)
 CHECK_REQ_SCALAR_INT_CLAUSE(NumTasks, OMPC_num_tasks)
Index: clang/tools/libclang/CIndex.cpp
===
--- clang/tools/libclang/CIndex.cpp
+++ clang/tools/libclang/CIndex.cpp
@@ -2402,6 +2402,8 @@
 
 void OMPClauseEnqueue::VisitOMPCompareClause(const OMPCompareClause *) {}
 
+void OMPClauseEnqueue::VisitOMPFailClause(const OMPFailClause *) {}
+
 void OMPClauseEnqueue::VisitOMPSeqCstClause(const OMPSeqCstClause *) {}
 
 void OMPClauseEnqueue::VisitOMPAcqRelClause(const OMPAcqRelClause *) {}
Index: clang/test/OpenMP/atomic_messages.cpp
===
--- clang/test/OpenMP/atomic_messages.cpp
+++ clang/test/OpenMP/atomic_messages.cpp
@@ -958,6 +958,24 @@
 // expected-error@+1 {{directive '#pragma omp atomic' cannot contain more than one 'capture' clause}}
 #pragma omp atomic compare compare capture capture
   { v = a; if (a > b) a = b; }
+// expected-error@+1 {{expected 'compare' clause with the 'fail' modifier}}
+#pragma omp atomic fail(seq_cst)
+  if(v == a) { v = a; }
+// expected-error@+1 {{expected '(' after 'fail'}}
+#pragma omp atomic compare fail
+  if(v < a) { v = a; }
+// expected-error@+1 {{expected a memory order clause}}
+#pragma omp atomic compare fail(capture)
+  if(v < a) { v = a; }
+ // expected-error@+2 {{expected ')'}}
+ // expected-note@+1 {{to match this '('}}
+#pragma omp atomic compare fail(seq_cst | acquire)
+  if(v < a) { v = a; }
+// expected-error@+1 {{directive '#pragma omp atomic' cannot contain more than one 'fail' clause}}
+#pragma omp atomic compare fail(relaxed) fail(seq_cst)
+  if(v < a) { v = a; }
+
+
 #endif
   // expected-note@+1 {{in instantiation of function template specialization 'mixed' requested here}}
   return mixed();
Index: clang/test/OpenMP/atomic_ast_print.cpp
===
--- clang/test/OpenMP/atomic_ast_print.cpp
+++ clang/test/OpenMP/atomic_ast_print.cpp
@@ -226,6 +226,12 @@
   { v = a; if (a < b) { a = b; } }
 #pragma omp atomic compare capture hint(6)
   { v = a == 

[PATCH] D153701: [Clang] Implement P2718R0 "Lifetime extension in range-based for loops"

[Vlad Serebrennikov via Phabricator via cfe-commits]

Endill added a comment.

@yronglin We are sorry it takes so much time to get feedback. Richard and 
Hubert have little bandwidth for reviews. Others, including me, don't feel 
qualified to provide good feedback.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153701

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

[PATCH] D153701: [Clang] Implement P2718R0 "Lifetime extension in range-based for loops"

[Yurong via Phabricator via cfe-commits]

yronglin added a comment.

ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153701

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

[PATCH] D150930: [Driver] Accept and ignore -fno-lifetime-dse argument

[Bruno De Fraine via Phabricator via cfe-commits]

brunodf added a comment.

Hi,

I found this review request and I just want to comment that I find it strange 
that it was rejected.

@MaskRay I understand that using a compile_commands.json configured for gcc 
with clang-based tools is not a supported use case, but still the clang driver 
was explicitly designed to emulate gcc: "The clang tool is the compiler driver 
and front-end, which is designed to be a drop-in replacement for the gcc 
command" and "The 'clang' driver is designed to work as closely to GCC as 
possible to maximize portability." are quotes from 
https://clang.llvm.org/get_started.html
In that regard, `clang_ignored_gcc_optimization_f_Group` is logical and it 
includes many options that you cite like `-falign-jumps=`, `-falign-loops=`, 
`-fmerge-constants`, `-fschedule-insns`, etc.
Sure, more projects support clang directly now, but I was not aware there is a 
change in this policy, or that there is a "stop" on adding more options (in 
that case, it would be consistent that the documentation is adapted to say that 
clang is only drop-in compatible with some historic version of gcc).

In my view, the main objection to "accept and ignore" a GCC option is when the 
option provides some guarantees that clang/LLVM cannot uphold. For example, 
ignoring `-fno-strict-aliasing` would be dangerous if you actually carry out 
type-based aliasing optimizations, because programs that compile with it likely 
contain violations of the strict aliasing rules. It seems that 
`-fno-lifetime-dse` similarly intends to allow violating a language rule. I'm 
not aware if clang/LLVM contains optimization that exploit this language rule 
(since the option appears in the context of the LLVM code base itself, and 
because compiling this code base with clang itself is well tested in many 
configurations, I suspect not), but if it does (now or in the future), ignoring 
this option is dangerous.

Regards,
Bruno


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150930

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

[PATCH] D158824: [RISCV][MC] MC layer support for xcvmem and xcvelw extensions

[Liao Chunyu via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG71a7108ee91a: [RISCV][MC] MC layer support for xcvmem and 
xcvelw extensions (authored by liaolucy).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158824

Files:
  clang/test/Preprocessor/riscv-target-features.c
  llvm/docs/RISCVUsage.rst
  llvm/lib/Support/RISCVISAInfo.cpp
  llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp
  llvm/lib/Target/RISCV/Disassembler/RISCVDisassembler.cpp
  llvm/lib/Target/RISCV/MCTargetDesc/RISCVInstPrinter.cpp
  llvm/lib/Target/RISCV/MCTargetDesc/RISCVInstPrinter.h
  llvm/lib/Target/RISCV/MCTargetDesc/RISCVMCCodeEmitter.cpp
  llvm/lib/Target/RISCV/RISCVFeatures.td
  llvm/lib/Target/RISCV/RISCVInstrInfoXCV.td
  llvm/test/CodeGen/RISCV/attributes.ll
  llvm/test/MC/RISCV/attribute-arch.s
  llvm/test/MC/RISCV/corev/XCVelw-invalid.s
  llvm/test/MC/RISCV/corev/XCVelw-valid.s
  llvm/test/MC/RISCV/corev/XCVmem-invalid.s
  llvm/test/MC/RISCV/corev/XCVmem-valid.s
  llvm/unittests/Support/RISCVISAInfoTest.cpp

Index: llvm/unittests/Support/RISCVISAInfoTest.cpp
===
--- llvm/unittests/Support/RISCVISAInfoTest.cpp
+++ llvm/unittests/Support/RISCVISAInfoTest.cpp
@@ -712,7 +712,9 @@
 xcvalu  1.0
 xcvbi   1.0
 xcvbitmanip 1.0
+xcvelw  1.0
 xcvmac  1.0
+xcvmem  1.0
 xcvsimd 1.0
 xsfcie  1.0
 xsfvcp  1.0
Index: llvm/test/MC/RISCV/corev/XCVmem-valid.s
===
--- /dev/null
+++ llvm/test/MC/RISCV/corev/XCVmem-valid.s
@@ -0,0 +1,247 @@
+# RUN: llvm-mc -triple=riscv32 --mattr=+xcvmem -show-encoding %s \
+# RUN: | FileCheck %s --check-prefixes=CHECK-ENCODING,CHECK-INSTR
+# RUN: llvm-mc -filetype=obj -triple=riscv32 -mattr=+xcvmem < %s \
+# RUN: | llvm-objdump --mattr=+xcvmem -M no-aliases -d -r - \
+# RUN: | FileCheck --check-prefix=CHECK-INSTR %s
+# RUN: not llvm-mc -triple riscv32 %s 2>&1 \
+# RUN: | FileCheck -check-prefix=CHECK-NO-EXT %s
+
+cv.lb t0, (t1), 0
+# CHECK-INSTR: cv.lb t0, (t1), 0
+# CHECK-ENCODING: [0x8b,0x02,0x03,0x00]
+# CHECK-NO-EXT: instruction requires the following: 'XCVmem' (CORE-V Post-incrementing Load & Store){{$}}
+
+cv.lb a0, (a1), 2047
+# CHECK-INSTR: cv.lb a0, (a1), 2047
+# CHECK-ENCODING: [0x0b,0x85,0xf5,0x7f]
+# CHECK-NO-EXT: instruction requires the following: 'XCVmem' (CORE-V Post-incrementing Load & Store){{$}}
+
+cv.lb t0, (t1), t2
+# CHECK-INSTR: cv.lb t0, (t1), t2
+# CHECK-ENCODING: [0xab,0x32,0x73,0x00]
+# CHECK-NO-EXT: instruction requires the following: 'XCVmem' (CORE-V Post-incrementing Load & Store){{$}}
+
+cv.lb a0, (a1), a2
+# CHECK-INSTR: cv.lb a0, (a1), a2
+# CHECK-ENCODING: [0x2b,0xb5,0xc5,0x00]
+# CHECK-NO-EXT: instruction requires the following: 'XCVmem' (CORE-V Post-incrementing Load & Store){{$}}
+
+cv.lb t0, t2(t1)
+# CHECK-INSTR: cv.lb t0, t2(t1)
+# CHECK-ENCODING: [0xab,0x32,0x73,0x08]
+# CHECK-NO-EXT: instruction requires the following: 'XCVmem' (CORE-V Post-incrementing Load & Store){{$}}
+
+cv.lb a0, a2(a1)
+# CHECK-INSTR: cv.lb a0, a2(a1)
+# CHECK-ENCODING: [0x2b,0xb5,0xc5,0x08]
+# CHECK-NO-EXT: instruction requires the following: 'XCVmem' (CORE-V Post-incrementing Load & Store){{$}}
+
+cv.lbu t0, (t1), 0
+# CHECK-INSTR: cv.lbu t0, (t1), 0
+# CHECK-ENCODING: [0x8b,0x42,0x03,0x00]
+# CHECK-NO-EXT: instruction requires the following: 'XCVmem' (CORE-V Post-incrementing Load & Store){{$}}
+
+cv.lbu a0, (a1), 2047
+# CHECK-INSTR: cv.lbu a0, (a1), 2047
+# CHECK-ENCODING: [0x0b,0xc5,0xf5,0x7f]
+# CHECK-NO-EXT: instruction requires the following: 'XCVmem' (CORE-V Post-incrementing Load & Store){{$}}
+
+cv.lbu t0, (t1), t2
+# CHECK-INSTR: cv.lbu t0, (t1), t2
+# CHECK-ENCODING: [0xab,0x32,0x73,0x10]
+# CHECK-NO-EXT: instruction requires the following: 'XCVmem' (CORE-V Post-incrementing Load & Store){{$}}
+
+cv.lbu a0, (a1), a2
+# CHECK-INSTR: cv.lbu a0, (a1), a2
+# CHECK-ENCODING: [0x2b,0xb5,0xc5,0x10]
+# CHECK-NO-EXT: instruction requires the following: 'XCVmem' (CORE-V Post-incrementing Load & Store){{$}}
+
+cv.lbu t0, t2(t1)
+# CHECK-INSTR: cv.lbu t0, t2(t1)
+# CHECK-ENCODING: [0xab,0x32,0x73,0x18]
+# CHECK-NO-EXT: instruction requires the following: 'XCVmem' (CORE-V Post-incrementing Load & Store){{$}}
+
+cv.lbu a0, a2(a1)
+# CHECK-INSTR: cv.lbu a0, a2(a1)
+# CHECK-ENCODING: [0x2b,0xb5,0xc5,0x18]
+# CHECK-NO-EXT: instruction requires the following: 'XCVmem' (CORE-V Post-incrementing Load & Store){{$}}
+
+cv.lh t0, (t1), 0
+# CHECK-INSTR: cv.lh t0, (t1), 0
+# CHECK-ENCODING: [0x8b,0x12,0x03,0x00]
+# CHECK-NO-EXT: instruction requires the following: 'XCVmem' (CORE-V Post-incrementing Load & Store){{$}}
+
+cv.lh a0, (a1), 2047
+# CHECK-INSTR: cv.lh a0, (a1), 2047
+# CHECK-ENCODING: 

[PATCH] D158824: [RISCV][MC] MC layer support for xcvmem and xcvelw extensions

[Craig Topper via Phabricator via cfe-commits]

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

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158824

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

[PATCH] D144006: [DebugMetadata][DwarfDebug] Support function-local types in lexical block scopes (4/7)

[Vladislav Dzhidzhoev via Phabricator via cfe-commits]

dzhidzhoev added a comment.

In D144006#4656399 , @aeubanks wrote:

> In D144006#4656383 , @dzhidzhoev 
> wrote:
>
>> In D144006#4656371 , @aeubanks 
>> wrote:
>>
>>> Is there any issue with mixing IR built with 
>>> `LLVMContext::enableDebugTypeODRUniquing()` and without? I'm suspecting 
>>> that ThinLTO is mixing Rust and Clang IR which set that differently.
>>
>> Apparently, it's an issue with ThinLTO that might be caused by a change in 
>> MetadataLoader https://github.com/llvm/llvm-project/pull/68986.
>> Is there a way to get a repro like this one 
>> https://bugs.chromium.org/p/chromium/issues/detail?id=1487910#c3 if it's not 
>> too complicated?
>
> I've shared the repro tar with your commit email address, sorry it's so big. 
> I tried linking again with the revert and now it looks like the debug info 
> verifier is firing instead. Do you also need a repro tar with everything 
> built with the revert?

Thank you so much! I think that's enough.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144006

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

[PATCH] D158824: [RISCV][MC] MC layer support for xcvmem and xcvelw extensions

[Liao Chunyu via Phabricator via cfe-commits]

liaolucy updated this revision to Diff 558107.
liaolucy added a comment.

1. use a custom parser to parse Register-Register load/store
2. Rebase

Thanks


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158824

Files:
  clang/test/Preprocessor/riscv-target-features.c
  llvm/docs/RISCVUsage.rst
  llvm/lib/Support/RISCVISAInfo.cpp
  llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp
  llvm/lib/Target/RISCV/Disassembler/RISCVDisassembler.cpp
  llvm/lib/Target/RISCV/MCTargetDesc/RISCVInstPrinter.cpp
  llvm/lib/Target/RISCV/MCTargetDesc/RISCVInstPrinter.h
  llvm/lib/Target/RISCV/MCTargetDesc/RISCVMCCodeEmitter.cpp
  llvm/lib/Target/RISCV/RISCVFeatures.td
  llvm/lib/Target/RISCV/RISCVInstrInfoXCV.td
  llvm/test/CodeGen/RISCV/attributes.ll
  llvm/test/MC/RISCV/attribute-arch.s
  llvm/test/MC/RISCV/corev/XCVelw-invalid.s
  llvm/test/MC/RISCV/corev/XCVelw-valid.s
  llvm/test/MC/RISCV/corev/XCVmem-invalid.s
  llvm/test/MC/RISCV/corev/XCVmem-valid.s
  llvm/unittests/Support/RISCVISAInfoTest.cpp

Index: llvm/unittests/Support/RISCVISAInfoTest.cpp
===
--- llvm/unittests/Support/RISCVISAInfoTest.cpp
+++ llvm/unittests/Support/RISCVISAInfoTest.cpp
@@ -712,7 +712,9 @@
 xcvalu  1.0
 xcvbi   1.0
 xcvbitmanip 1.0
+xcvelw  1.0
 xcvmac  1.0
+xcvmem  1.0
 xcvsimd 1.0
 xsfcie  1.0
 xsfvcp  1.0
Index: llvm/test/MC/RISCV/corev/XCVmem-valid.s
===
--- /dev/null
+++ llvm/test/MC/RISCV/corev/XCVmem-valid.s
@@ -0,0 +1,247 @@
+# RUN: llvm-mc -triple=riscv32 --mattr=+xcvmem -show-encoding %s \
+# RUN: | FileCheck %s --check-prefixes=CHECK-ENCODING,CHECK-INSTR
+# RUN: llvm-mc -filetype=obj -triple=riscv32 -mattr=+xcvmem < %s \
+# RUN: | llvm-objdump --mattr=+xcvmem -M no-aliases -d -r - \
+# RUN: | FileCheck --check-prefix=CHECK-INSTR %s
+# RUN: not llvm-mc -triple riscv32 %s 2>&1 \
+# RUN: | FileCheck -check-prefix=CHECK-NO-EXT %s
+
+cv.lb t0, (t1), 0
+# CHECK-INSTR: cv.lb t0, (t1), 0
+# CHECK-ENCODING: [0x8b,0x02,0x03,0x00]
+# CHECK-NO-EXT: instruction requires the following: 'XCVmem' (CORE-V Post-incrementing Load & Store){{$}}
+
+cv.lb a0, (a1), 2047
+# CHECK-INSTR: cv.lb a0, (a1), 2047
+# CHECK-ENCODING: [0x0b,0x85,0xf5,0x7f]
+# CHECK-NO-EXT: instruction requires the following: 'XCVmem' (CORE-V Post-incrementing Load & Store){{$}}
+
+cv.lb t0, (t1), t2
+# CHECK-INSTR: cv.lb t0, (t1), t2
+# CHECK-ENCODING: [0xab,0x32,0x73,0x00]
+# CHECK-NO-EXT: instruction requires the following: 'XCVmem' (CORE-V Post-incrementing Load & Store){{$}}
+
+cv.lb a0, (a1), a2
+# CHECK-INSTR: cv.lb a0, (a1), a2
+# CHECK-ENCODING: [0x2b,0xb5,0xc5,0x00]
+# CHECK-NO-EXT: instruction requires the following: 'XCVmem' (CORE-V Post-incrementing Load & Store){{$}}
+
+cv.lb t0, t2(t1)
+# CHECK-INSTR: cv.lb t0, t2(t1)
+# CHECK-ENCODING: [0xab,0x32,0x73,0x08]
+# CHECK-NO-EXT: instruction requires the following: 'XCVmem' (CORE-V Post-incrementing Load & Store){{$}}
+
+cv.lb a0, a2(a1)
+# CHECK-INSTR: cv.lb a0, a2(a1)
+# CHECK-ENCODING: [0x2b,0xb5,0xc5,0x08]
+# CHECK-NO-EXT: instruction requires the following: 'XCVmem' (CORE-V Post-incrementing Load & Store){{$}}
+
+cv.lbu t0, (t1), 0
+# CHECK-INSTR: cv.lbu t0, (t1), 0
+# CHECK-ENCODING: [0x8b,0x42,0x03,0x00]
+# CHECK-NO-EXT: instruction requires the following: 'XCVmem' (CORE-V Post-incrementing Load & Store){{$}}
+
+cv.lbu a0, (a1), 2047
+# CHECK-INSTR: cv.lbu a0, (a1), 2047
+# CHECK-ENCODING: [0x0b,0xc5,0xf5,0x7f]
+# CHECK-NO-EXT: instruction requires the following: 'XCVmem' (CORE-V Post-incrementing Load & Store){{$}}
+
+cv.lbu t0, (t1), t2
+# CHECK-INSTR: cv.lbu t0, (t1), t2
+# CHECK-ENCODING: [0xab,0x32,0x73,0x10]
+# CHECK-NO-EXT: instruction requires the following: 'XCVmem' (CORE-V Post-incrementing Load & Store){{$}}
+
+cv.lbu a0, (a1), a2
+# CHECK-INSTR: cv.lbu a0, (a1), a2
+# CHECK-ENCODING: [0x2b,0xb5,0xc5,0x10]
+# CHECK-NO-EXT: instruction requires the following: 'XCVmem' (CORE-V Post-incrementing Load & Store){{$}}
+
+cv.lbu t0, t2(t1)
+# CHECK-INSTR: cv.lbu t0, t2(t1)
+# CHECK-ENCODING: [0xab,0x32,0x73,0x18]
+# CHECK-NO-EXT: instruction requires the following: 'XCVmem' (CORE-V Post-incrementing Load & Store){{$}}
+
+cv.lbu a0, a2(a1)
+# CHECK-INSTR: cv.lbu a0, a2(a1)
+# CHECK-ENCODING: [0x2b,0xb5,0xc5,0x18]
+# CHECK-NO-EXT: instruction requires the following: 'XCVmem' (CORE-V Post-incrementing Load & Store){{$}}
+
+cv.lh t0, (t1), 0
+# CHECK-INSTR: cv.lh t0, (t1), 0
+# CHECK-ENCODING: [0x8b,0x12,0x03,0x00]
+# CHECK-NO-EXT: instruction requires the following: 'XCVmem' (CORE-V Post-incrementing Load & Store){{$}}
+
+cv.lh a0, (a1), 2047
+# CHECK-INSTR: cv.lh a0, (a1), 2047
+# CHECK-ENCODING: [0x0b,0x95,0xf5,0x7f]
+# CHECK-NO-EXT: instruction 

[PATCH] D144006: [DebugMetadata][DwarfDebug] Support function-local types in lexical block scopes (4/7)

[Vladislav Dzhidzhoev via Phabricator via cfe-commits]

dzhidzhoev added a comment.

In D144006#4656728 , @jmorse wrote:

> Hi,
>
> Just to note that we've been seeing LTO crashes as a result of 
> rG3b449bd46a11a 
>  (now 
> reverted on trunk), which @StephenTozer has kindly reduced down to:
>
>   https://gist.github.com/jmorse/b0248c3c9f9195487ffd7c7431a8d15e
>
> Which triggers the assertion: DwarfDebug.cpp:2335: virtual void 
> llvm::DwarfDebug::endFunctionImpl(const llvm::MachineFunction *): Assertion 
> `LScopes.getAbstractScopesList().size() == NumAbstractSubprograms && 
> "getOrCreateAbstractScope() inserted an abstract subprogram scope"' failed. 
> If you revert-the-revert rG6beddd668 
>  that is.
>
> I haven't familiarised myself with this patch series (while greatly 
> appreciating that it exists!), so perhaps this is already obvious but: This 
> particular assertion is a check that no additional lexical scopes are 
> discovered during DWARF emission that weren't found during the earlier 
> building of the lexical-scopes-map, which enumerates all 
> scopes/inlining-chains for all instructions' DebugLocs. If any more 
> unexpectedly appear after that, I believe there's a risk that a container for 
> lexical scopes gets reallocated, causing random crashes. I see there are now 
> types in the retainedNodes field for DISubprograms with "scope" fields (EDIT: 
> !53 in the reproducer), I imagine that the discovery of those lexical scopes 
> which weren't reachable from instructions might be causing the assertion.

I appreciate your help! Can I ask you how to reproduce that crash? When I run 
llc or llvm-lto on the gist, the assertion is triggered

  Assertion failed: (((LinkageName.empty() || DeclLinkageName.empty()) || 
LinkageName == DeclLinkageName) && "decl has a linkage name and it is 
different"), function applySubprogramDefinitionAttributes, file DwarfUnit.cpp, 
line 1222.

that persists even with this commit reverted. Are you sure that the reproducer 
was correctly reduced?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144006

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

[PATCH] D150083: [clang-format] ObjCPropertyAttributeOrder to sort ObjC property attributes

[Owen Pan via Phabricator via cfe-commits]

owenpan added a comment.

BTW, it may be simpler and more efficient to use a set (e.g. `llvm::SmallSet`) 
for `Indices`, especially if we don't need/want to handle duplicate attributes 
that have a value. (See D150083#inline-1551778 
.)


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

https://reviews.llvm.org/D150083

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

[PATCH] D150083: [clang-format] ObjCPropertyAttributeOrder to sort ObjC property attributes

[Owen Pan via Phabricator via cfe-commits]

owenpan added a comment.

Thank you for your patience! There are still a few comments not done from the 
previous round.

In D150083#4655528 , @owenpan wrote:

> See also D153228 .

Would this patch have a similar performance issue?




Comment at: clang/lib/Format/ObjCPropertyAttributeOrderFixer.cpp:73
+  Tok = Tok->Next;
+  if (!Tok->Next->is(tok::identifier)) {
+// If we hit any other kind of token, just bail. It's unusual/illegal.





Comment at: clang/lib/Format/ObjCPropertyAttributeOrderFixer.cpp:93
+  const auto SortOrderMax = Style.ObjCPropertyAttributeOrder.size();
+  const auto SortIndex = [&](const StringRef ) -> unsigned {
+auto I = SortOrderMap.find(Needle);





Comment at: clang/lib/Format/ObjCPropertyAttributeOrderFixer.cpp:143
+const SourceManager , const AdditionalKeywords ,
+tooling::Replacements , const FormatToken *const Tok) const {
+  // Expect `property` to be the very next token or else just bail early.





Comment at: clang/lib/Format/ObjCPropertyAttributeOrderFixer.cpp:185
+
+for (const auto *Tok = First; Tok && Tok != Last && Tok->Next;
+ Tok = Tok->Next) {





Comment at: clang/lib/Format/ObjCPropertyAttributeOrderFixer.cpp:113
+
+  // Deduplicate the attributes.
+  Indices.erase(std::unique(Indices.begin(), Indices.end(),

jaredgrubb wrote:
> owenpan wrote:
> > Is it valid in Objective-C to have duplicate attributes?
> It's silly, but clang doesn't seem to care. I tried this, so duplicate was 
> ok, but contradiction was flagged:
> ```
> @property (strong, nonatomic, nonatomic) X *X;   // duplicate, but no error
> @property (strong, nonatomic, weak) Y *y;   // error: Property attributes 
> 'strong' and 'weak' are mutually exclusive
> ```
> 
> I wasn't sure whether to do this, but went that way since that's what "sort 
> include files" did. However, it seems like an odd corner case so I'd be ok 
> removing this uniquing if you prefer.
We should keep the duplicates if clang accepts them. What happens to e.g. 
`@property(x=a, y=b, x=a, y=c)`?



Comment at: clang/lib/Format/ObjCPropertyAttributeOrderFixer.cpp:184
+  for (AnnotatedLine *Line : AnnotatedLines) {
+if (!Line->Affected || Line->InPPDirective)
+  continue;

jaredgrubb wrote:
> owenpan wrote:
> > Why not `InPPDirective`?
> I copy-pasted this from `LeftRightQualifierAlignmentFixer::analyze`, which I 
> used as a template since I'm still getting used to the codebase. I wasn't 
> sure whether this was important, so I left it in. But I don't think I have a 
> good reason. 
> 
> I've added a new test case `SortsInPPDirective` that spot-checks some macro 
> definition examples (which did fail unless this `Line->InPPDirective` check 
> was removed, as expected.).
What about `Line->Type != LT_ObjCProperty` I suggested?



Comment at: clang/lib/Format/ObjCPropertyAttributeOrderFixer.cpp:190
+  continue;
+
+const auto *Last = Line->Last;

jaredgrubb wrote:
> owenpan wrote:
> > Must `@property` be the first non-comment tokens of an unwrapped line in 
> > Objective-C? And at most one `@property` per line?
> I can't think of any token that should precede a `@property`. Aka, I don't 
> think there are any `__attribute__` that can fill that slot. 
> 
> You could have `@optional` or `@required` floating around between 
> property/method declarations. I've added a test-case for a `@property` that 
> is preceded by these tokens and proved that the reordering is handled 
> correctly.
> 
> As for multiple properties in one line, I've never seen a codebase with that. 
> clang-format does split them already.
I asked the questions because if yes, we could move on to the next line before 
hitting the last token of the current line.


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

https://reviews.llvm.org/D150083

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

[PATCH] D149573: [Clang][C++23] Implement core language changes from P1467R9 extended floating-point types and standard names

[M. Zeeshan Siddiqui via Phabricator via cfe-commits]

codemzs updated this revision to Diff 558099.
codemzs added a comment.

Hi @tahonermann,

I've just pushed a new diff with tests for `va_arg` and `...`, ensuring 
promotion rules are intact.

Also, I've made sure `getFloatingTypeOrder` returns `FRCR_Unordered` only when 
we're dealing with both operands as C++23 extended floating point types and 
they are unordered. I've added tests like `_Float16 f16_val_1 = 1.0bf16; // 
expected-error {{cannot initialize a variable of type '_Float16' with an rvalue 
of type '__bf16'}}` to cover these cases. If you see any potential issues here, 
I'm all ears and ready to make adjustments.

Thanks a lot for reviewing this, especially with your busy schedule.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149573

Files:
  clang/include/clang/AST/ASTContext.h
  clang/include/clang/AST/Type.h
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Lex/LiteralSupport.h
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/SMTConv.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/AST/StmtPrinter.cpp
  clang/lib/AST/Type.cpp
  clang/lib/Frontend/InitPreprocessor.cpp
  clang/lib/Lex/LiteralSupport.cpp
  clang/lib/Sema/Sema.cpp
  clang/lib/Sema/SemaCast.cpp
  clang/lib/Sema/SemaChecking.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaOverload.cpp
  clang/test/CodeGenCXX/cxx23-fp-ext-std-names-p1467r9.cpp
  clang/test/CodeGenCXX/cxx23-vector-bfloat16.cpp
  clang/test/Sema/cxx23-fp-ext-std-names-p1467r9.cpp

Index: clang/test/Sema/cxx23-fp-ext-std-names-p1467r9.cpp
===
--- /dev/null
+++ clang/test/Sema/cxx23-fp-ext-std-names-p1467r9.cpp
@@ -0,0 +1,505 @@
+// RUN: %clang_cc1 -fsyntax-only -std=c++23 -target-feature +fullbf16 -verify -ast-dump %s | FileCheck %s
+#include 
+_Float16 f16_val_1 = 1.0bf16; // expected-error {{cannot initialize a variable of type '_Float16' with an rvalue of type '__bf16'}}
+_Float16 f16_val_2 = 1.0f; // expected-error {{cannot initialize a variable of type '_Float16' with an rvalue of type 'float'}}
+_Float16 f16_val_3 = 1.0; // expected-error {{cannot initialize a variable of type '_Float16' with an rvalue of type 'double'}}
+_Float16 f16_val_4 = 1.0l; // expected-error {{cannot initialize a variable of type '_Float16' with an rvalue of type 'long double'}}
+_Float16 f16_val_6 = 1.0f16;
+//CHECK:  VarDecl {{.*}} f16_val_6 '_Float16' cinit
+//CHECK-NEXT: FloatingLiteral {{.*}} '_Float16' 1.00e+00
+_Float16 f16_val_7 = static_cast<_Float16>(1.0bf16); // expected-error {{static_cast from '__bf16' to '_Float16' is not allowed}}
+_Float16 f16_val_8 = static_cast<_Float16>(1.0f);
+//CHECK:  VarDecl {{.*}} f16_val_8 '_Float16' cinit
+//CHECK-NEXT: CXXStaticCastExpr {{.*}} '_Float16' static_cast<_Float16> 
+//CHECK-NEXT: FloatingLiteral {{.*}} 'float' 1.00e+00
+_Float16 f16_val_9 = static_cast<_Float16>(1.0);
+//CHECK:  VarDecl {{.*}} f16_val_9 '_Float16' cinit
+//CHECK-NEXT: CXXStaticCastExpr {{.*}} '_Float16' static_cast<_Float16> 
+//CHECK-NEXT: FloatingLiteral {{.*}} 'double' 1.00e+00
+_Float16 f16_val_10 = static_cast<_Float16>(1.0l);
+//CHECK:  VarDecl {{.*}} f16_val_10 '_Float16' cinit
+//CHECK-NEXT: CXXStaticCastExpr {{.*}} '_Float16' static_cast<_Float16> 
+//CHECK-NEXT: FloatingLiteral {{.*}} 'long double' 1.00e+00
+_Float16 f16_val_11 = static_cast<_Float16>(1.0f16);
+//CHECK:  VarDecl {{.*}} f16_val_11 '_Float16' cinit
+//CHECK-NEXT: CXXStaticCastExpr {{.*}} '_Float16' static_cast<_Float16> 
+//CHECK-NEXT: FloatingLiteral {{.*}} '_Float16' 1.00e+00
+
+decltype(0.0BF16) bf16_val_1 = 1.0f16; // expected-error {{cannot initialize a variable of type 'decltype(0.BF16)' (aka '__bf16') with an rvalue of type '_Float16'}}
+decltype(0.0BF16) bf16_val_2 = 1.0f; // expected-error {{cannot initialize a variable of type 'decltype(0.BF16)' (aka '__bf16') with an rvalue of type 'float'}}
+decltype(0.0BF16) bf16_val_3 = 1.0; // expected-error {{cannot initialize a variable of type 'decltype(0.BF16)' (aka '__bf16') with an rvalue of type 'double'}}
+decltype(0.0BF16) bf16_val_4 = 1.0l; // expected-error {{cannot initialize a variable of type 'decltype(0.BF16)' (aka '__bf16') with an rvalue of type 'long double'}}
+decltype(0.0BF16) bf16_val_5 = 1.0bf16;
+//CHECK:  VarDecl {{.*}} bf16_val_5 'decltype(0.BF16)':'__bf16' cinit
+//CHECK-NEXT: FloatingLiteral {{.*}} '__bf16' 1.00e+00
+
+decltype(0.0BF16) bf16_val_6 = static_cast(1.0f16); // expected-error {{static_cast from '_Float16' to 'decltype(0.BF16)' (aka '__bf16') is not allowed}}
+decltype(0.0BF16) bf16_val_7 = static_cast(1.0f);
+//CHECK:  VarDecl {{.*}} bf16_val_7 'decltype(0.BF16)':'__bf16' cinit
+//CHECK-NEXT: CXXStaticCastExpr {{.*}} 'decltype(0.BF16)':'__bf16' static_cast 
+//CHECK-NEXT: FloatingLiteral {{.*}} 'float' 1.00e+00
+decltype(0.0BF16) bf16_val_8 = static_cast(1.0);

[PATCH] D148474: [Clang] Fix ResolveConstructorOverload to not select a conversion function if we are going use copy elision

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman accepted this revision.
aaron.ballman added a comment.

The changes look reasonable to me (@rsmith had a lot of comments, but I think 
you addressed them; it would be nice if he could confirm), but should 
definitely come with a release note. So LGTM modulo those nits and any 
last-minute comments from Richard.




Comment at: clang/lib/Sema/SemaTemplateInstantiate.cpp:803
 << FD << Active->InstantiationRange;
+  } else if (ClassTemplateDecl *CTD = dyn_cast(D)) {
+Diags.Report(Active->PointOfInstantiation,




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

https://reviews.llvm.org/D148474

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

[PATCH] D148474: [Clang] Fix ResolveConstructorOverload to not select a conversion function if we are going use copy elision

[Dimitry Andric via Phabricator via cfe-commits]

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

Yes, please. :)


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

https://reviews.llvm.org/D148474

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

[PATCH] D141700: AMDGPU: Move enqueued block handling into clang

[Matt Arsenault via Phabricator via cfe-commits]

arsenm updated this revision to Diff 558095.
arsenm added a comment.

Drop bitcode auto upgrade handling


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

https://reviews.llvm.org/D141700

Files:
  clang/lib/CodeGen/Targets/AMDGPU.cpp
  clang/test/CodeGenOpenCL/amdgpu-enqueue-kernel-linking.cl
  clang/test/CodeGenOpenCL/amdgpu-enqueue-kernel.cl
  llvm/docs/AMDGPUUsage.rst
  llvm/lib/IR/AutoUpgrade.cpp
  llvm/lib/IR/CMakeLists.txt
  llvm/lib/Target/AMDGPU/AMDGPU.h
  llvm/lib/Target/AMDGPU/AMDGPUExportKernelRuntimeHandles.cpp
  llvm/lib/Target/AMDGPU/AMDGPUHSAMetadataStreamer.cpp
  llvm/lib/Target/AMDGPU/AMDGPUHSAMetadataStreamer.h
  llvm/lib/Target/AMDGPU/AMDGPUOpenCLEnqueuedBlockLowering.cpp
  llvm/lib/Target/AMDGPU/AMDGPUTargetMachine.cpp
  llvm/lib/Target/AMDGPU/CMakeLists.txt
  llvm/test/CodeGen/AMDGPU/amdgpu-export-kernel-runtime-handles.ll
  llvm/test/CodeGen/AMDGPU/enqueue-kernel.ll
  llvm/test/CodeGen/AMDGPU/hsa-metadata-from-llvm-ir-full.ll
  llvm/test/CodeGen/AMDGPU/llc-pipeline.ll

Index: llvm/test/CodeGen/AMDGPU/llc-pipeline.ll
===
--- llvm/test/CodeGen/AMDGPU/llc-pipeline.ll
+++ llvm/test/CodeGen/AMDGPU/llc-pipeline.ll
@@ -37,7 +37,7 @@
 ; GCN-O0-NEXT:Dominator Tree Construction
 ; GCN-O0-NEXT:Basic Alias Analysis (stateless AA impl)
 ; GCN-O0-NEXT:Function Alias Analysis Results
-; GCN-O0-NEXT:Lower OpenCL enqueued blocks
+; GCN-O0-NEXT:Externalize enqueued block runtime handles
 ; GCN-O0-NEXT:Lower uses of LDS variables from non-kernel functions
 ; GCN-O0-NEXT:FunctionPass Manager
 ; GCN-O0-NEXT:  Expand Atomic instructions
@@ -178,7 +178,7 @@
 ; GCN-O1-NEXT:Dominator Tree Construction
 ; GCN-O1-NEXT:Basic Alias Analysis (stateless AA impl)
 ; GCN-O1-NEXT:Function Alias Analysis Results
-; GCN-O1-NEXT:Lower OpenCL enqueued blocks
+; GCN-O1-NEXT:Externalize enqueued block runtime handles
 ; GCN-O1-NEXT:Lower uses of LDS variables from non-kernel functions
 ; GCN-O1-NEXT:AMDGPU Attributor
 ; GCN-O1-NEXT:  FunctionPass Manager
@@ -445,7 +445,7 @@
 ; GCN-O1-OPTS-NEXT:Dominator Tree Construction
 ; GCN-O1-OPTS-NEXT:Basic Alias Analysis (stateless AA impl)
 ; GCN-O1-OPTS-NEXT:Function Alias Analysis Results
-; GCN-O1-OPTS-NEXT:Lower OpenCL enqueued blocks
+; GCN-O1-OPTS-NEXT:Externalize enqueued block runtime handles
 ; GCN-O1-OPTS-NEXT:Lower uses of LDS variables from non-kernel functions
 ; GCN-O1-OPTS-NEXT:AMDGPU Attributor
 ; GCN-O1-OPTS-NEXT:  FunctionPass Manager
@@ -736,7 +736,7 @@
 ; GCN-O2-NEXT:Dominator Tree Construction
 ; GCN-O2-NEXT:Basic Alias Analysis (stateless AA impl)
 ; GCN-O2-NEXT:Function Alias Analysis Results
-; GCN-O2-NEXT:Lower OpenCL enqueued blocks
+; GCN-O2-NEXT:Externalize enqueued block runtime handles
 ; GCN-O2-NEXT:Lower uses of LDS variables from non-kernel functions
 ; GCN-O2-NEXT:AMDGPU Attributor
 ; GCN-O2-NEXT:  FunctionPass Manager
@@ -1037,7 +1037,7 @@
 ; GCN-O3-NEXT:Dominator Tree Construction
 ; GCN-O3-NEXT:Basic Alias Analysis (stateless AA impl)
 ; GCN-O3-NEXT:Function Alias Analysis Results
-; GCN-O3-NEXT:Lower OpenCL enqueued blocks
+; GCN-O3-NEXT:Externalize enqueued block runtime handles
 ; GCN-O3-NEXT:Lower uses of LDS variables from non-kernel functions
 ; GCN-O3-NEXT:AMDGPU Attributor
 ; GCN-O3-NEXT:  FunctionPass Manager
Index: llvm/test/CodeGen/AMDGPU/hsa-metadata-from-llvm-ir-full.ll
===
--- llvm/test/CodeGen/AMDGPU/hsa-metadata-from-llvm-ir-full.ll
+++ llvm/test/CodeGen/AMDGPU/hsa-metadata-from-llvm-ir-full.ll
@@ -14,7 +14,8 @@
 %struct.B = type { ptr addrspace(1) }
 %opencl.clk_event_t = type opaque
 
-@__test_block_invoke_kernel_runtime_handle = external addrspace(1) externally_initialized constant ptr addrspace(1)
+@__test_block_invoke_kernel_runtime_handle = external addrspace(1) externally_initialized constant ptr addrspace(1), section ".amdgpu.kernel.runtime.handle"
+@not.a.handle = external addrspace(1) externally_initialized constant ptr addrspace(1)
 
 ; CHECK:  ---
 ; CHECK-NEXT: amdhsa.kernels:
@@ -1678,7 +1679,7 @@
 ; CHECK:  .name:   __test_block_invoke_kernel
 ; CHECK:  .symbol: __test_block_invoke_kernel.kd
 define amdgpu_kernel void @__test_block_invoke_kernel(
-<{ i32, i32, ptr, ptr addrspace(1), i8 }> %arg) #1
+<{ i32, i32, ptr, ptr addrspace(1), i8 }> %arg) #1 !associated !112
 !kernel_arg_addr_space !1 !kernel_arg_access_qual !2 !kernel_arg_type !110
 !kernel_arg_base_type !110 !kernel_arg_type_qual !4 {
   ret void
@@ -1734,6 +1735,29 @@
   ret void
 }
 
+; Make sure the device_enqueue_symbol is not reported
+; CHECK: - .args:   []
+; CHECK-NEXT: .group_segment_fixed_size: 0
+; CHECK-NEXT: 

[PATCH] D141700: AMDGPU: Move enqueued block handling into clang

[Matt Arsenault via Phabricator via cfe-commits]

arsenm added inline comments.



Comment at: llvm/lib/IR/CMakeLists.txt:84
   Demangle
+  TransformUtils
+

This introduces a circular dependency between LLVMCore and TransformUtils. 
Options are:

1. Move appendToUsed into Module
2. Don't bother with bitcode compatibility for this
3. Avoid depending on llvm.used. I know I tried to do this but it was so long 
ago I don't remember how I ended up on this solution 


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

https://reviews.llvm.org/D141700

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

[PATCH] D156370: [clang-format] Fix bug with parsing of function/variable names.

[Owen Pan via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGa852869398af: [clang-format] Fix a bug in parsing 
function/variable names (authored by gedare, committed by owenpan).

Changed prior to commit:
  https://reviews.llvm.org/D156370?vs=558061=558088#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156370

Files:
  clang/lib/Format/TokenAnnotator.cpp
  clang/unittests/Format/FormatTest.cpp
  clang/unittests/Format/TokenAnnotatorTest.cpp


Index: clang/unittests/Format/TokenAnnotatorTest.cpp
===
--- clang/unittests/Format/TokenAnnotatorTest.cpp
+++ clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -2313,6 +2313,7 @@
 TEST_F(TokenAnnotatorTest, UnderstandsAttributes) {
   auto Tokens = annotate("bool foo __attribute__((unused));");
   ASSERT_EQ(Tokens.size(), 10u) << Tokens;
+  EXPECT_TOKEN(Tokens[1], tok::identifier, TT_StartOfName);
   EXPECT_TOKEN(Tokens[3], tok::l_paren, TT_AttributeLParen);
   EXPECT_TOKEN(Tokens[4], tok::l_paren, TT_Unknown);
   EXPECT_TOKEN(Tokens[6], tok::r_paren, TT_Unknown);
@@ -2323,6 +2324,22 @@
   EXPECT_TOKEN(Tokens[3], tok::l_paren, TT_AttributeLParen);
   EXPECT_TOKEN(Tokens[5], tok::r_paren, TT_AttributeRParen);
 
+  Tokens = annotate("bool __attribute__((unused)) foo;");
+  ASSERT_EQ(Tokens.size(), 10u) << Tokens;
+  EXPECT_TOKEN(Tokens[2], tok::l_paren, TT_AttributeLParen);
+  EXPECT_TOKEN(Tokens[3], tok::l_paren, TT_Unknown);
+  EXPECT_TOKEN(Tokens[5], tok::r_paren, TT_Unknown);
+  EXPECT_TOKEN(Tokens[6], tok::r_paren, TT_AttributeRParen);
+  EXPECT_TOKEN(Tokens[7], tok::identifier, TT_StartOfName);
+
+  Tokens = annotate("void __attribute__((x)) Foo();");
+  ASSERT_EQ(Tokens.size(), 12u) << Tokens;
+  EXPECT_TOKEN(Tokens[2], tok::l_paren, TT_AttributeLParen);
+  EXPECT_TOKEN(Tokens[3], tok::l_paren, TT_Unknown);
+  EXPECT_TOKEN(Tokens[5], tok::r_paren, TT_Unknown);
+  EXPECT_TOKEN(Tokens[6], tok::r_paren, TT_AttributeRParen);
+  EXPECT_TOKEN(Tokens[7], tok::identifier, TT_FunctionDeclarationName);
+
   FormatStyle Style = getLLVMStyle();
   Style.AttributeMacros.push_back("FOO");
   Tokens = annotate("bool foo FOO(unused);", Style);
Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -16394,8 +16394,10 @@
 
   verifyFormat("int f ();", SpaceFuncDecl);
   verifyFormat("void f(int a, T b) {}", SpaceFuncDecl);
+  verifyFormat("void __attribute__((asdf)) f(int a, T b) {}", SpaceFuncDecl);
   verifyFormat("A::A() : a(1) {}", SpaceFuncDecl);
   verifyFormat("void f () __attribute__((asdf));", SpaceFuncDecl);
+  verifyFormat("void __attribute__((asdf)) f ();", SpaceFuncDecl);
   verifyFormat("#define A(x) x", SpaceFuncDecl);
   verifyFormat("#define A (x) x", SpaceFuncDecl);
   verifyFormat("#if defined(x)\n"
@@ -16429,8 +16431,10 @@
 
   verifyFormat("int f();", SpaceFuncDef);
   verifyFormat("void f (int a, T b) {}", SpaceFuncDef);
+  verifyFormat("void __attribute__((asdf)) f (int a, T b) {}", SpaceFuncDef);
   verifyFormat("A::A() : a(1) {}", SpaceFuncDef);
   verifyFormat("void f() __attribute__((asdf));", SpaceFuncDef);
+  verifyFormat("void __attribute__((asdf)) f();", SpaceFuncDef);
   verifyFormat("#define A(x) x", SpaceFuncDef);
   verifyFormat("#define A (x) x", SpaceFuncDef);
   verifyFormat("#if defined(x)\n"
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -2246,8 +2246,9 @@
  
PreviousNotConst->MatchingParen->Previous->isNot(tok::kw_template);
 }
 
-if (PreviousNotConst->is(tok::r_paren) &&
-PreviousNotConst->is(TT_TypeDeclarationParen)) {
+if ((PreviousNotConst->is(tok::r_paren) &&
+ PreviousNotConst->is(TT_TypeDeclarationParen)) ||
+PreviousNotConst->is(TT_AttributeRParen)) {
   return true;
 }
 


Index: clang/unittests/Format/TokenAnnotatorTest.cpp
===
--- clang/unittests/Format/TokenAnnotatorTest.cpp
+++ clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -2313,6 +2313,7 @@
 TEST_F(TokenAnnotatorTest, UnderstandsAttributes) {
   auto Tokens = annotate("bool foo __attribute__((unused));");
   ASSERT_EQ(Tokens.size(), 10u) << Tokens;
+  EXPECT_TOKEN(Tokens[1], tok::identifier, TT_StartOfName);
   EXPECT_TOKEN(Tokens[3], tok::l_paren, TT_AttributeLParen);
   EXPECT_TOKEN(Tokens[4], tok::l_paren, TT_Unknown);
   EXPECT_TOKEN(Tokens[6], tok::r_paren, TT_Unknown);
@@ -2323,6 +2324,22 @@
   EXPECT_TOKEN(Tokens[3], tok::l_paren, TT_AttributeLParen);
   EXPECT_TOKEN(Tokens[5], tok::r_paren, TT_AttributeRParen);
 
+  Tokens = annotate("bool __attribute__((unused)) foo;");
+  

[PATCH] D148474: [Clang] Fix ResolveConstructorOverload to not select a conversion function if we are going use copy elision

[Shafik Yaghmour via Phabricator via cfe-commits]

shafik added a comment.

ping @dim @rsmith @aaron.ballman


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

https://reviews.llvm.org/D148474

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

[PATCH] D143967: [DebugInfo][BPF] Add 'btf:type_tag' annotation in DWARF

[Eduard Zingerman via Phabricator via cfe-commits]

eddyz87 added a comment.

In D143967#4656478 , @dblaikie wrote:

> Fair enough - all seems a bit unfortunate to be pushing these attributes 
> through to places they don't technically apply to (both complicates the 
> implementation, and might be confusing to users).

Thank you for taking a look!
I'll wait till the last part of the commit stack (BPF-specific) is ironed out 
and apply all three patches.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143967

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

[PATCH] D143967: [DebugInfo][BPF] Add 'btf:type_tag' annotation in DWARF

[Eduard Zingerman via Phabricator via cfe-commits]

eddyz87 updated this revision to Diff 558086.
eddyz87 marked 2 inline comments as done.
eddyz87 added a comment.

Rebase, fixes for review feedback.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143967

Files:
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/lib/CodeGen/CGDebugInfo.h
  clang/test/CodeGen/attr-btf_type_tag-circular.c
  clang/test/CodeGen/attr-btf_type_tag-const.c
  clang/test/CodeGen/attr-btf_type_tag-func-ptr.c
  clang/test/CodeGen/attr-btf_type_tag-func.c
  clang/test/CodeGen/attr-btf_type_tag-restrict.c
  clang/test/CodeGen/attr-btf_type_tag-similar-type.c
  clang/test/CodeGen/attr-btf_type_tag-typedef-field.c
  clang/test/CodeGen/attr-btf_type_tag-var.c
  clang/test/CodeGen/attr-btf_type_tag-void.c
  clang/test/CodeGen/attr-btf_type_tag-volatile.c
  llvm/include/llvm/IR/DIBuilder.h
  llvm/lib/IR/DIBuilder.cpp

Index: llvm/lib/IR/DIBuilder.cpp
===
--- llvm/lib/IR/DIBuilder.cpp
+++ llvm/lib/IR/DIBuilder.cpp
@@ -342,6 +342,17 @@
 Annotations);
 }
 
+DIDerivedType *
+DIBuilder::createAnnotationsPlaceholder(DIType *Ty, DINodeArray Annotations) {
+  auto *RetTy =
+  DIDerivedType::getTemporary(
+  VMContext, dwarf::DW_TAG_LLVM_annotation, "", nullptr, 0, nullptr, Ty,
+  0, 0, 0, std::nullopt, DINode::FlagZero, nullptr, Annotations)
+  .release();
+  trackIfUnresolved(RetTy);
+  return RetTy;
+}
+
 DIDerivedType *DIBuilder::createFriend(DIType *Ty, DIType *FriendTy) {
   assert(Ty && "Invalid type!");
   assert(FriendTy && "Invalid friend type!");
Index: llvm/include/llvm/IR/DIBuilder.h
===
--- llvm/include/llvm/IR/DIBuilder.h
+++ llvm/include/llvm/IR/DIBuilder.h
@@ -294,6 +294,9 @@
  DINode::DIFlags Flags = DINode::FlagZero,
  DINodeArray Annotations = nullptr);
 
+DIDerivedType *createAnnotationsPlaceholder(DIType *Ty,
+DINodeArray Annotations);
+
 /// Create debugging information entry for a 'friend'.
 DIDerivedType *createFriend(DIType *Ty, DIType *FriendTy);
 
Index: clang/test/CodeGen/attr-btf_type_tag-volatile.c
===
--- /dev/null
+++ clang/test/CodeGen/attr-btf_type_tag-volatile.c
@@ -0,0 +1,18 @@
+// RUN: %clang_cc1 \
+// RUN:   -triple %itanium_abi_triple -debug-info-kind=limited \
+// RUN:   -S -emit-llvm -o - %s | FileCheck %s
+
+// See attr-btf_type_tag-const.c for reasoning behind this test.
+// Alternatively, see the following method:
+//   CGDebugInfo::CreateType(const BTFTagAttributedType, llvm::DIFile)
+
+#define __tag1 __attribute__((btf_type_tag("tag1")))
+
+volatile int foo;
+typeof(foo) __tag1 bar;
+
+// CHECK: ![[#]] = distinct !DIGlobalVariable(name: "bar", {{.*}}, type: ![[L1:[0-9]+]], {{.*}})
+// CHECK: ![[L1]] = !DIDerivedType(tag: DW_TAG_volatile_type, baseType: ![[L2:[0-9]+]])
+// CHECK: ![[L2]] = !DIBasicType(name: "int", size: [[#]], {{.*}}, annotations: ![[L3:[0-9]+]])
+// CHECK: ![[L3]] = !{![[L4:[0-9]+]]}
+// CHECK: ![[L4]] = !{!"btf:type_tag", !"tag1"}
Index: clang/test/CodeGen/attr-btf_type_tag-void.c
===
--- /dev/null
+++ clang/test/CodeGen/attr-btf_type_tag-void.c
@@ -0,0 +1,12 @@
+// RUN: %clang_cc1 -triple %itanium_abi_triple -debug-info-kind=limited -S -emit-llvm -o - %s | FileCheck %s
+
+#define __tag1 __attribute__((btf_type_tag("tag1")))
+void __tag1 *g;
+
+// CHECK: distinct !DIGlobalVariable(name: "g", scope: ![[#]], file: ![[#]], line: [[#]], type: ![[L1:[0-9]+]], isLocal: false, isDefinition: true)
+// CHECK: ![[L1]] = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: ![[L2:[0-9]+]], size: [[#]], annotations: ![[L3:[0-9]+]])
+// CHECK: ![[L2]] = !DIBasicType(tag: DW_TAG_unspecified_type, name: "void", annotations: ![[L4:[0-9]+]])
+// CHECK: ![[L4]] = !{![[L5:[0-9]+]]}
+// CHECK: ![[L5]] = !{!"btf:type_tag", !"tag1"}
+// CHECK: ![[L3]] = !{![[L6:[0-9]+]]}
+// CHECK: ![[L6]] = !{!"btf_type_tag", !"tag1"}
Index: clang/test/CodeGen/attr-btf_type_tag-var.c
===
--- clang/test/CodeGen/attr-btf_type_tag-var.c
+++ clang/test/CodeGen/attr-btf_type_tag-var.c
@@ -21,23 +21,30 @@
 const int __tag1 __tag2 volatile * const __tag3  __tag4  volatile * __tag5  __tag6 const volatile * g;
 #endif
 
-// CHECK:  distinct !DIGlobalVariable(name: "g", scope: ![[#]], file: ![[#]], line: [[#]], type: ![[L6:[0-9]+]]
-// CHECK:  ![[L6]] = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: ![[L7:[0-9]+]], size: [[#]], annotations: ![[L22:[0-9]+]]
-// CHECK:  ![[L7]] = !DIDerivedType(tag: DW_TAG_const_type, baseType: ![[L8:[0-9]+]]
-// CHECK:  ![[L8]] = !DIDerivedType(tag: DW_TAG_volatile_type, 

[PATCH] D149573: [Clang][C++23] Implement core language changes from P1467R9 extended floating-point types and standard names

[M. Zeeshan Siddiqui via Phabricator via cfe-commits]

codemzs updated this revision to Diff 558083.
codemzs set the repository for this revision to rG LLVM Github Monorepo.
codemzs added a comment.

Synced to the main and resolved merge conflicts, updated tests.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149573

Files:
  clang/include/clang/AST/ASTContext.h
  clang/include/clang/AST/Type.h
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Lex/LiteralSupport.h
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/SMTConv.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/AST/StmtPrinter.cpp
  clang/lib/AST/Type.cpp
  clang/lib/Frontend/InitPreprocessor.cpp
  clang/lib/Lex/LiteralSupport.cpp
  clang/lib/Sema/Sema.cpp
  clang/lib/Sema/SemaCast.cpp
  clang/lib/Sema/SemaChecking.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaOverload.cpp
  clang/test/CodeGenCXX/cxx23-fp-ext-std-names-p1467r9.cpp
  clang/test/CodeGenCXX/cxx23-vector-bfloat16.cpp
  clang/test/Sema/cxx23-fp-ext-std-names-p1467r9.cpp

Index: clang/test/Sema/cxx23-fp-ext-std-names-p1467r9.cpp
===
--- /dev/null
+++ clang/test/Sema/cxx23-fp-ext-std-names-p1467r9.cpp
@@ -0,0 +1,465 @@
+// RUN: %clang_cc1 -fsyntax-only -std=c++23 -target-feature +fullbf16 -verify -ast-dump %s | FileCheck %s
+
+_Float16 f16_val_1 = 1.0bf16; // expected-error {{cannot initialize a variable of type '_Float16' with an rvalue of type '__bf16'}}
+_Float16 f16_val_2 = 1.0f; // expected-error {{cannot initialize a variable of type '_Float16' with an rvalue of type 'float'}}
+_Float16 f16_val_3 = 1.0; // expected-error {{cannot initialize a variable of type '_Float16' with an rvalue of type 'double'}}
+_Float16 f16_val_4 = 1.0l; // expected-error {{cannot initialize a variable of type '_Float16' with an rvalue of type 'long double'}}
+_Float16 f16_val_6 = 1.0f16;
+//CHECK:  VarDecl {{.*}} f16_val_6 '_Float16' cinit
+//CHECK-NEXT: FloatingLiteral {{.*}} '_Float16' 1.00e+00
+_Float16 f16_val_7 = static_cast<_Float16>(1.0bf16); // expected-error {{static_cast from '__bf16' to '_Float16' is not allowed}}
+_Float16 f16_val_8 = static_cast<_Float16>(1.0f);
+//CHECK:  VarDecl {{.*}} f16_val_8 '_Float16' cinit
+//CHECK-NEXT: CXXStaticCastExpr {{.*}} '_Float16' static_cast<_Float16> 
+//CHECK-NEXT: FloatingLiteral {{.*}} 'float' 1.00e+00
+_Float16 f16_val_9 = static_cast<_Float16>(1.0);
+//CHECK:  VarDecl {{.*}} f16_val_9 '_Float16' cinit
+//CHECK-NEXT: CXXStaticCastExpr {{.*}} '_Float16' static_cast<_Float16> 
+//CHECK-NEXT: FloatingLiteral {{.*}} 'double' 1.00e+00
+_Float16 f16_val_10 = static_cast<_Float16>(1.0l);
+//CHECK:  VarDecl {{.*}} f16_val_10 '_Float16' cinit
+//CHECK-NEXT: CXXStaticCastExpr {{.*}} '_Float16' static_cast<_Float16> 
+//CHECK-NEXT: FloatingLiteral {{.*}} 'long double' 1.00e+00
+_Float16 f16_val_11 = static_cast<_Float16>(1.0f16);
+//CHECK:  VarDecl {{.*}} f16_val_11 '_Float16' cinit
+//CHECK-NEXT: CXXStaticCastExpr {{.*}} '_Float16' static_cast<_Float16> 
+//CHECK-NEXT: FloatingLiteral {{.*}} '_Float16' 1.00e+00
+
+decltype(0.0BF16) bf16_val_1 = 1.0f16; // expected-error {{cannot initialize a variable of type 'decltype(0.BF16)' (aka '__bf16') with an rvalue of type '_Float16'}}
+decltype(0.0BF16) bf16_val_2 = 1.0f; // expected-error {{cannot initialize a variable of type 'decltype(0.BF16)' (aka '__bf16') with an rvalue of type 'float'}}
+decltype(0.0BF16) bf16_val_3 = 1.0; // expected-error {{cannot initialize a variable of type 'decltype(0.BF16)' (aka '__bf16') with an rvalue of type 'double'}}
+decltype(0.0BF16) bf16_val_4 = 1.0l; // expected-error {{cannot initialize a variable of type 'decltype(0.BF16)' (aka '__bf16') with an rvalue of type 'long double'}}
+decltype(0.0BF16) bf16_val_5 = 1.0bf16;
+//CHECK:  VarDecl {{.*}} bf16_val_5 'decltype(0.BF16)':'__bf16' cinit
+//CHECK-NEXT: FloatingLiteral {{.*}} '__bf16' 1.00e+00
+
+decltype(0.0BF16) bf16_val_6 = static_cast(1.0f16); // expected-error {{static_cast from '_Float16' to 'decltype(0.BF16)' (aka '__bf16') is not allowed}}
+decltype(0.0BF16) bf16_val_7 = static_cast(1.0f);
+//CHECK:  VarDecl {{.*}} bf16_val_7 'decltype(0.BF16)':'__bf16' cinit
+//CHECK-NEXT: CXXStaticCastExpr {{.*}} 'decltype(0.BF16)':'__bf16' static_cast 
+//CHECK-NEXT: FloatingLiteral {{.*}} 'float' 1.00e+00
+decltype(0.0BF16) bf16_val_8 = static_cast(1.0);
+//CHECK:  VarDecl {{.*}} bf16_val_8 'decltype(0.BF16)':'__bf16' cinit
+//CHECK-NEXT: CXXStaticCastExpr {{.*}} 'decltype(0.BF16)':'__bf16' static_cast 
+//CHECK-NEXT: FloatingLiteral {{.*}} 'double' 1.00e+00
+decltype(0.0BF16) bf16_val_9 = static_cast(1.0l);
+//CHECK:  VarDecl {{.*}} bf16_val_9 'decltype(0.BF16)':'__bf16' cinit
+//CHECK-NEXT: CXXStaticCastExpr {{.*}} 'decltype(0.BF16)':'__bf16' static_cast 
+//CHECK-NEXT: FloatingLiteral {{.*}} 'long double' 1.00e+00
+decltype(0.0BF16) 

[PATCH] D150083: [clang-format] ObjCPropertyAttributeOrder to sort ObjC property attributes

[Jared Grubb via Phabricator via cfe-commits]

jaredgrubb added a comment.

From my perspective, this patch is ready to go! Please let me know if there's 
anything more I can do to improve the patch! (@owenpan thanks so much for your 
help so far!)


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

https://reviews.llvm.org/D150083

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

[PATCH] D144006: [DebugMetadata][DwarfDebug] Support function-local types in lexical block scopes (4/7)

[Jeremy Morse via Phabricator via cfe-commits]

jmorse added subscribers: StephenTozer, jmorse.
jmorse added a comment.

Hi,

Just to note that we've been seeing LTO crashes as a result of rG3b449bd46a11a 
 (now 
reverted on trunk), which @StephenTozer has kindly reduced down to:

  https://gist.github.com/jmorse/b0248c3c9f9195487ffd7c7431a8d15e

Which triggers the assertion: DwarfDebug.cpp:2335: virtual void 
llvm::DwarfDebug::endFunctionImpl(const llvm::MachineFunction *): Assertion 
`LScopes.getAbstractScopesList().size() == NumAbstractSubprograms && 
"getOrCreateAbstractScope() inserted an abstract subprogram scope"' failed. If 
you revert-the-revert rG6beddd668 
 that is.

I haven't familiarised myself with this patch series (while greatly 
appreciating that it exists!), so perhaps this is already obvious but: This 
particular assertion is a check that no additional lexical scopes are 
discovered during DWARF emission that weren't found during the earlier building 
of the lexical-scopes-map, which enumerates all scopes/inlining-chains for all 
instructions' DebugLocs. If any more unexpectedly appear after that, I believe 
there's a risk that a container for lexical scopes gets reallocated, causing 
random crashes. I see there are now types in the retainedNodes field for 
DISubprograms with "scope" fields, I imagine that the discovery of those 
lexical scopes which weren't reachable from instructions might be causing the 
assertion.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144006

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

[PATCH] D123235: [OpenMP] atomic compare fail : Parser & AST support

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.



Comment at: clang/include/clang/AST/OpenMPClause.h:2554
+  break;
+case llvm::omp::OMPC_adjust_args:
+case llvm::omp::OMPC_affinity:

I think all these cases are unexpected and must be terminated with 
llvm_unreachable


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

https://reviews.llvm.org/D123235

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

[PATCH] D154774: [Sema] Respect instantiated-from template's VisibilityAttr for two implicit/explicit instantiation cases

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added inline comments.



Comment at: clang/lib/AST/Decl.cpp:380
+->getTemplatedDecl()
+->hasAttr();
 }

rjmccall wrote:
> Okay, this change seems wrong.  A visibility attribute on an explicit 
> specialization or instantiation should definitely override everything else.  
> A visibility attribute on a template pattern should only override other 
> visibility information that we would derive from that pattern, like the 
> visibility of the template parameters; it should not override visibility from 
> the template arguments.  You probably need this function to return an enum 
> with three cases: (1) factor in both template arguments and template 
> parameters, (2) factor in only template arguments, and (3) factor in nothing.
> 
> Also, the bug here doesn't seem to be specific to function templates, and you 
> should fix the code for all three paths (function templates, class templates, 
> and variable templates).  Basically we're universally mishandling visibility 
> attributes on template patterns for templates with non-type template 
> parameters with hidden visibility.
I have added test coverage to `test51` in `visibility.cpp` and studied GCC's 
behavior.

Sent https://github.com/llvm/llvm-project/pull/72092 to ignore visibility 
effects from template parameters. It will bring our behavior closer to GCC.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154774

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

[PATCH] D33531: Clang-tidy readability: avoid const value return

[Piotr Zegar via Phabricator via cfe-commits]

PiotrZSL abandoned this revision.
PiotrZSL marked 4 inline comments as done.
PiotrZSL added a comment.

Obsolete, this check is already added.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D33531

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

[PATCH] D144429: [clang-tidy] Add bugprone-chained-comparison check

[Piotr Zegar via Phabricator via cfe-commits]

PiotrZSL added a comment.

Ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144429

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

[PATCH] D158657: [clang-tidy] Fix false-positives in misc-static-assert caused by non-constexpr variables

[Piotr Zegar via Phabricator via cfe-commits]

PiotrZSL added a comment.

Ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158657

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

[PATCH] D101617: [clang-tidy] Tweak diag ranges for bugprone-sizeof-expression

[Piotr Zegar via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG0e55fef0e98f: [clang-tidy] Tweak diag ranges for 
bugprone-sizeof-expression (authored by njames93, committed by PiotrZSL).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101617

Files:
  clang-tools-extra/clang-tidy/bugprone/SizeofExpressionCheck.cpp
  clang-tools-extra/clangd/test/diagnostics-tidy.test
  clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
  
clang-tools-extra/test/clang-tidy/checkers/bugprone/sizeof-expression-warn-on-sizeof-pointer-to-aggregate.cpp
  clang-tools-extra/test/clang-tidy/checkers/bugprone/sizeof-expression.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/bugprone/sizeof-expression.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/bugprone/sizeof-expression.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone/sizeof-expression.cpp
@@ -77,7 +77,7 @@
   sum += sizeof(LEN + 1);
   // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(K)'
   sum += sizeof(sum, LEN);
-  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(..., ...)'
+  // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: suspicious usage of 'sizeof(..., ...)'
   sum += sizeof(AsBool());
   // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof()' on an expression that results in an integer
   sum += sizeof(AsInt());
@@ -103,41 +103,41 @@
   sum += sizeof(LEN + - + -sizeof(X));
   // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(sizeof(...))'
   sum += sizeof(char) / sizeof(char);
-  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of sizeof pointer 'sizeof(T)/sizeof(T)'
+  // CHECK-MESSAGES: :[[@LINE-1]]:23: warning: suspicious usage of sizeof pointer 'sizeof(T)/sizeof(T)'
   sum += sizeof(A) / sizeof(S);
-  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(...)/sizeof(...)'; numerator is not a multiple of denominator
+  // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: suspicious usage of 'sizeof(...)/sizeof(...)'; numerator is not a multiple of denominator
   sum += sizeof(char) / sizeof(int);
-  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(...)/sizeof(...)'; numerator is not a multiple of denominator
+  // CHECK-MESSAGES: :[[@LINE-1]]:23: warning: suspicious usage of 'sizeof(...)/sizeof(...)'; numerator is not a multiple of denominator
   sum += sizeof(char) / sizeof(A);
-  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(...)/sizeof(...)'; numerator is not a multiple of denominator
+  // CHECK-MESSAGES: :[[@LINE-1]]:23: warning: suspicious usage of 'sizeof(...)/sizeof(...)'; numerator is not a multiple of denominator
   sum += sizeof(B[0]) / sizeof(A);
-  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(...)/sizeof(...)'; numerator is not a multiple of denominator
+  // CHECK-MESSAGES: :[[@LINE-1]]:23: warning: suspicious usage of 'sizeof(...)/sizeof(...)'; numerator is not a multiple of denominator
   sum += sizeof(ptr) / sizeof(char);
-  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of sizeof pointer 'sizeof(T*)/sizeof(T)'
+  // CHECK-MESSAGES: :[[@LINE-1]]:22: warning: suspicious usage of sizeof pointer 'sizeof(T*)/sizeof(T)'
   sum += sizeof(ptr) / sizeof(ptr[0]);
-  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of sizeof pointer 'sizeof(T*)/sizeof(T)'
+  // CHECK-MESSAGES: :[[@LINE-1]]:22: warning: suspicious usage of sizeof pointer 'sizeof(T*)/sizeof(T)'
   sum += sizeof(ptr) / sizeof(char*);
-  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of sizeof pointer 'sizeof(P*)/sizeof(Q*)'
+  // CHECK-MESSAGES: :[[@LINE-1]]:22: warning: suspicious usage of sizeof pointer 'sizeof(P*)/sizeof(Q*)'
   sum += sizeof(ptr) / sizeof(void*);
-  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of sizeof pointer 'sizeof(P*)/sizeof(Q*)'
+  // CHECK-MESSAGES: :[[@LINE-1]]:22: warning: suspicious usage of sizeof pointer 'sizeof(P*)/sizeof(Q*)'
   sum += sizeof(ptr) / sizeof(const void volatile*);
-  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of sizeof pointer 'sizeof(P*)/sizeof(Q*)'
+  // CHECK-MESSAGES: :[[@LINE-1]]:22: warning: suspicious usage of sizeof pointer 'sizeof(P*)/sizeof(Q*)'
   sum += sizeof(ptr) / sizeof(char);
-  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of sizeof pointer 'sizeof(T*)/sizeof(T)'
+  // CHECK-MESSAGES: :[[@LINE-1]]:22: warning: suspicious usage of sizeof pointer 'sizeof(T*)/sizeof(T)'
   sum += sizeof(ptr) / sizeof(ptr[0]);
-  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of sizeof pointer 'sizeof(T*)/sizeof(T)'
+  // CHECK-MESSAGES: :[[@LINE-1]]:22: warning: suspicious usage of sizeof pointer 'sizeof(T*)/sizeof(T)'
   sum += sizeof(int) * 

[PATCH] D125949: [clang-tidy] modernize-avoid-bind: Fix crash when method name is not a simple identifier

[Piotr Zegar via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGd867f668672d: [clang-tidy] modernize-avoid-bind: Fix 
handling of operators (authored by jspam, committed by PiotrZSL).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125949

Files:
  clang-tools-extra/clang-tidy/modernize/AvoidBindCheck.cpp
  clang-tools-extra/test/clang-tidy/checkers/modernize/avoid-bind.cpp


Index: clang-tools-extra/test/clang-tidy/checkers/modernize/avoid-bind.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/modernize/avoid-bind.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/modernize/avoid-bind.cpp
@@ -43,6 +43,7 @@
 struct D {
   D() = default;
   void operator()(int x, int y) const {}
+  operator bool() const { return true; }
 
   void MemberFunction(int x) {}
   int MemberFunctionWithReturn(int x) {}
@@ -342,6 +343,7 @@
 struct E {
   void MemberFunction(int x) {}
   int MemberFunctionWithReturn(int x) {}
+  int operator()(int x, int y) const { return x + y; }
 
   void testMemberFunctions() {
 D *d;
@@ -379,6 +381,26 @@
 auto HHH = std::bind(::MemberFunctionWithReturn, _1, 1);
 // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: prefer a lambda to std::bind
 // CHECK-FIXES: auto HHH = [](auto && PH1) { return 
PH1->MemberFunctionWithReturn(1); };
+
+auto III = std::bind(::operator(), d, 1, 2);
+// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: prefer a lambda to std::bind
+// CHECK-FIXES: auto III = [d] { (*d)(1, 2); }
+
+auto JJJ = std::bind(::operator(), , 1, 2);
+// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: prefer a lambda to std::bind
+// CHECK-FIXES: auto JJJ = [ObjectPtr = ] { (*ObjectPtr)(1, 2); }
+
+auto KKK = std::bind(::operator(), _1, 1, 2);
+// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: prefer a lambda to std::bind
+// CHECK-FIXES: auto KKK = [](auto && PH1) { (*PH1)(1, 2); };
+
+auto LLL = std::bind(::operator bool, d);
+// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: prefer a lambda to std::bind
+// CHECK-FIXES: auto LLL = [d] { return d->operator bool(); }
+
+auto MMM = std::bind(::operator(), this, 1, 2);
+// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: prefer a lambda to std::bind
+// CHECK-FIXES: auto MMM = [this] { return (*this)(1, 2); }
   }
 };
 
Index: clang-tools-extra/clang-tidy/modernize/AvoidBindCheck.cpp
===
--- clang-tools-extra/clang-tidy/modernize/AvoidBindCheck.cpp
+++ clang-tools-extra/clang-tidy/modernize/AvoidBindCheck.cpp
@@ -691,12 +691,15 @@
 const auto *MethodDecl = dyn_cast(LP.Callable.Decl);
 const BindArgument  = FunctionCallArgs.front();
 
-if (!isa(ignoreTemporariesAndPointers(ObjPtr.E))) {
-  Stream << ObjPtr.UsageIdentifier;
-  Stream << "->";
+if (MethodDecl->getOverloadedOperator() == OO_Call) {
+  Stream << "(*" << ObjPtr.UsageIdentifier << ')';
+} else {
+  if (!isa(ignoreTemporariesAndPointers(ObjPtr.E))) {
+Stream << ObjPtr.UsageIdentifier;
+Stream << "->";
+  }
+  Stream << MethodDecl->getNameAsString();
 }
-
-Stream << MethodDecl->getName();
   } else {
 switch (LP.Callable.CE) {
 case CE_Var:


Index: clang-tools-extra/test/clang-tidy/checkers/modernize/avoid-bind.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/modernize/avoid-bind.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/modernize/avoid-bind.cpp
@@ -43,6 +43,7 @@
 struct D {
   D() = default;
   void operator()(int x, int y) const {}
+  operator bool() const { return true; }
 
   void MemberFunction(int x) {}
   int MemberFunctionWithReturn(int x) {}
@@ -342,6 +343,7 @@
 struct E {
   void MemberFunction(int x) {}
   int MemberFunctionWithReturn(int x) {}
+  int operator()(int x, int y) const { return x + y; }
 
   void testMemberFunctions() {
 D *d;
@@ -379,6 +381,26 @@
 auto HHH = std::bind(::MemberFunctionWithReturn, _1, 1);
 // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: prefer a lambda to std::bind
 // CHECK-FIXES: auto HHH = [](auto && PH1) { return PH1->MemberFunctionWithReturn(1); };
+
+auto III = std::bind(::operator(), d, 1, 2);
+// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: prefer a lambda to std::bind
+// CHECK-FIXES: auto III = [d] { (*d)(1, 2); }
+
+auto JJJ = std::bind(::operator(), , 1, 2);
+// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: prefer a lambda to std::bind
+// CHECK-FIXES: auto JJJ = [ObjectPtr = ] { (*ObjectPtr)(1, 2); }
+
+auto KKK = std::bind(::operator(), _1, 1, 2);
+// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: prefer a lambda to std::bind
+// CHECK-FIXES: auto KKK = [](auto && PH1) { (*PH1)(1, 2); };
+
+auto LLL = std::bind(::operator bool, d);
+// CHECK-MESSAGES: 

[PATCH] D123235: [OpenMP] atomic compare fail : Parser & AST support

[Sunil K via Phabricator via cfe-commits]

koops updated this revision to Diff 558076.
koops added a comment.

Removing default from switch statements.


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

https://reviews.llvm.org/D123235

Files:
  clang/include/clang/AST/OpenMPClause.h
  clang/include/clang/AST/RecursiveASTVisitor.h
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Basic/OpenMPKinds.def
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/OpenMPClause.cpp
  clang/lib/AST/StmtProfile.cpp
  clang/lib/Basic/OpenMPKinds.cpp
  clang/lib/CodeGen/CGStmtOpenMP.cpp
  clang/lib/Parse/ParseOpenMP.cpp
  clang/lib/Sema/SemaOpenMP.cpp
  clang/lib/Sema/TreeTransform.h
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTWriter.cpp
  clang/test/OpenMP/atomic_ast_print.cpp
  clang/test/OpenMP/atomic_messages.cpp
  clang/tools/libclang/CIndex.cpp
  flang/lib/Semantics/check-omp-structure.cpp
  llvm/include/llvm/Frontend/OpenMP/OMP.td

Index: llvm/include/llvm/Frontend/OpenMP/OMP.td
===
--- llvm/include/llvm/Frontend/OpenMP/OMP.td
+++ llvm/include/llvm/Frontend/OpenMP/OMP.td
@@ -209,6 +209,7 @@
 def OMPC_Update : Clause<"update"> { let clangClass = "OMPUpdateClause"; }
 def OMPC_Capture : Clause<"capture"> { let clangClass = "OMPCaptureClause"; }
 def OMPC_Compare : Clause<"compare"> { let clangClass = "OMPCompareClause"; }
+def OMPC_Fail : Clause<"fail"> { let clangClass = "OMPFailClause"; }
 def OMPC_SeqCst : Clause<"seq_cst"> { let clangClass = "OMPSeqCstClause"; }
 def OMPC_AcqRel : Clause<"acq_rel"> { let clangClass = "OMPAcqRelClause"; }
 def OMPC_Acquire : Clause<"acquire"> { let clangClass = "OMPAcquireClause"; }
@@ -637,7 +638,8 @@
 VersionedClause,
 VersionedClause,
 VersionedClause,
-VersionedClause
+VersionedClause,
+VersionedClause
   ];
 }
 def OMP_Target : Directive<"target"> {
Index: flang/lib/Semantics/check-omp-structure.cpp
===
--- flang/lib/Semantics/check-omp-structure.cpp
+++ flang/lib/Semantics/check-omp-structure.cpp
@@ -2227,6 +2227,7 @@
 CHECK_SIMPLE_CLAUSE(Doacross, OMPC_doacross)
 CHECK_SIMPLE_CLAUSE(OmpxAttribute, OMPC_ompx_attribute)
 CHECK_SIMPLE_CLAUSE(OmpxBare, OMPC_ompx_bare)
+CHECK_SIMPLE_CLAUSE(Fail, OMPC_fail)
 
 CHECK_REQ_SCALAR_INT_CLAUSE(Grainsize, OMPC_grainsize)
 CHECK_REQ_SCALAR_INT_CLAUSE(NumTasks, OMPC_num_tasks)
Index: clang/tools/libclang/CIndex.cpp
===
--- clang/tools/libclang/CIndex.cpp
+++ clang/tools/libclang/CIndex.cpp
@@ -2402,6 +2402,8 @@
 
 void OMPClauseEnqueue::VisitOMPCompareClause(const OMPCompareClause *) {}
 
+void OMPClauseEnqueue::VisitOMPFailClause(const OMPFailClause *) {}
+
 void OMPClauseEnqueue::VisitOMPSeqCstClause(const OMPSeqCstClause *) {}
 
 void OMPClauseEnqueue::VisitOMPAcqRelClause(const OMPAcqRelClause *) {}
Index: clang/test/OpenMP/atomic_messages.cpp
===
--- clang/test/OpenMP/atomic_messages.cpp
+++ clang/test/OpenMP/atomic_messages.cpp
@@ -958,6 +958,24 @@
 // expected-error@+1 {{directive '#pragma omp atomic' cannot contain more than one 'capture' clause}}
 #pragma omp atomic compare compare capture capture
   { v = a; if (a > b) a = b; }
+// expected-error@+1 {{expected 'compare' clause with the 'fail' modifier}}
+#pragma omp atomic fail(seq_cst)
+  if(v == a) { v = a; }
+// expected-error@+1 {{expected '(' after 'fail'}}
+#pragma omp atomic compare fail
+  if(v < a) { v = a; }
+// expected-error@+1 {{expected a memory order clause}}
+#pragma omp atomic compare fail(capture)
+  if(v < a) { v = a; }
+ // expected-error@+2 {{expected ')'}}
+ // expected-note@+1 {{to match this '('}}
+#pragma omp atomic compare fail(seq_cst | acquire)
+  if(v < a) { v = a; }
+// expected-error@+1 {{directive '#pragma omp atomic' cannot contain more than one 'fail' clause}}
+#pragma omp atomic compare fail(relaxed) fail(seq_cst)
+  if(v < a) { v = a; }
+
+
 #endif
   // expected-note@+1 {{in instantiation of function template specialization 'mixed' requested here}}
   return mixed();
Index: clang/test/OpenMP/atomic_ast_print.cpp
===
--- clang/test/OpenMP/atomic_ast_print.cpp
+++ clang/test/OpenMP/atomic_ast_print.cpp
@@ -226,6 +226,16 @@
   { v = a; if (a < b) { a = b; } }
 #pragma omp atomic compare capture hint(6)
   { v = a == b; if (v) a = c; }
+#pragma omp atomic compare fail(acq_rel)
+  { if (a < c) { a = c; } }
+#pragma omp atomic compare fail(acquire)
+  { if (a < c) { a = c; } }
+#pragma omp atomic compare fail(release)
+  { if (a < c) { a = c; } }
+#pragma omp atomic compare fail(relaxed)
+  { if (a < c) { a = c; } }
+#pragma omp atomic compare fail(seq_cst)
+  { if (a < c) { a = c; } }
 #endif
   return T();
 }
@@ -1099,6 +1109,16 @@
   { v = a; if (a < b) { a = b; } }
 #pragma omp atomic 

[PATCH] D138846: MC/DC in LLVM Source-Based Code Coverage: LLVM back-end and compiler-rt

[Alan Phipps via Phabricator via cfe-commits]

alanphipps added a comment.

In D138846#4656594 , @hans wrote:

> Just a heads up that we're seeing crashes in the Rust compiler which bisects 
> to this change. Investigating in 
> https://bugs.chromium.org/p/chromium/issues/detail?id=1500558

Thank you for the repro!  This was a very straightforward fix; effectively the 
inlining of an instrumented function into multiple functions led to the 
creation of duplicate data variables, ultimately leading to a segfault in 
emitNamedata() when eraseFromParent() was called multiple times from the same 
NamePtr.  Prior to my change to abstract the creation of the data variable, 
there was no explicit check for this, but it was inadvertently avoided by 
checking that multiple counter sections were not created.

I created a pull request for the fix here: 
https://github.com/llvm/llvm-project/pull/71998


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138846

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

[PATCH] D147073: [Coverage] Handle invalid end location of an expression/statement.

[Zequan Wu via Phabricator via cfe-commits]

zequanwu added inline comments.



Comment at: clang/lib/CodeGen/CoverageMappingGen.cpp:607
+// compiler has broken.
+assert((!StartLoc || StartLoc->isValid()) && "Start location is not 
valid");
+assert((!EndLoc || EndLoc->isValid()) && "End location is not valid");

MaskRay wrote:
> Is this workaround still needed after 
> b0e61de7075942ef5ac8af9ca1ec918317f62152 (with a test 
> `clang/test/Coverage/unresolved-ctor-expr.cpp`)?
I think this workaround can be removed, verified it no longer crashes if 
reverting this change.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147073

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

[PATCH] D101617: [clang-tidy] Tweak diag ranges for bugprone-sizeof-expression

[Piotr Zegar via Phabricator via cfe-commits]

PiotrZSL updated this revision to Diff 558075.
PiotrZSL added a comment.

Rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101617

Files:
  clang-tools-extra/clang-tidy/bugprone/SizeofExpressionCheck.cpp
  clang-tools-extra/clangd/test/diagnostics-tidy.test
  clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
  
clang-tools-extra/test/clang-tidy/checkers/bugprone/sizeof-expression-warn-on-sizeof-pointer-to-aggregate.cpp
  clang-tools-extra/test/clang-tidy/checkers/bugprone/sizeof-expression.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/bugprone/sizeof-expression.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/bugprone/sizeof-expression.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone/sizeof-expression.cpp
@@ -77,7 +77,7 @@
   sum += sizeof(LEN + 1);
   // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(K)'
   sum += sizeof(sum, LEN);
-  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(..., ...)'
+  // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: suspicious usage of 'sizeof(..., ...)'
   sum += sizeof(AsBool());
   // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof()' on an expression that results in an integer
   sum += sizeof(AsInt());
@@ -103,41 +103,41 @@
   sum += sizeof(LEN + - + -sizeof(X));
   // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(sizeof(...))'
   sum += sizeof(char) / sizeof(char);
-  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of sizeof pointer 'sizeof(T)/sizeof(T)'
+  // CHECK-MESSAGES: :[[@LINE-1]]:23: warning: suspicious usage of sizeof pointer 'sizeof(T)/sizeof(T)'
   sum += sizeof(A) / sizeof(S);
-  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(...)/sizeof(...)'; numerator is not a multiple of denominator
+  // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: suspicious usage of 'sizeof(...)/sizeof(...)'; numerator is not a multiple of denominator
   sum += sizeof(char) / sizeof(int);
-  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(...)/sizeof(...)'; numerator is not a multiple of denominator
+  // CHECK-MESSAGES: :[[@LINE-1]]:23: warning: suspicious usage of 'sizeof(...)/sizeof(...)'; numerator is not a multiple of denominator
   sum += sizeof(char) / sizeof(A);
-  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(...)/sizeof(...)'; numerator is not a multiple of denominator
+  // CHECK-MESSAGES: :[[@LINE-1]]:23: warning: suspicious usage of 'sizeof(...)/sizeof(...)'; numerator is not a multiple of denominator
   sum += sizeof(B[0]) / sizeof(A);
-  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(...)/sizeof(...)'; numerator is not a multiple of denominator
+  // CHECK-MESSAGES: :[[@LINE-1]]:23: warning: suspicious usage of 'sizeof(...)/sizeof(...)'; numerator is not a multiple of denominator
   sum += sizeof(ptr) / sizeof(char);
-  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of sizeof pointer 'sizeof(T*)/sizeof(T)'
+  // CHECK-MESSAGES: :[[@LINE-1]]:22: warning: suspicious usage of sizeof pointer 'sizeof(T*)/sizeof(T)'
   sum += sizeof(ptr) / sizeof(ptr[0]);
-  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of sizeof pointer 'sizeof(T*)/sizeof(T)'
+  // CHECK-MESSAGES: :[[@LINE-1]]:22: warning: suspicious usage of sizeof pointer 'sizeof(T*)/sizeof(T)'
   sum += sizeof(ptr) / sizeof(char*);
-  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of sizeof pointer 'sizeof(P*)/sizeof(Q*)'
+  // CHECK-MESSAGES: :[[@LINE-1]]:22: warning: suspicious usage of sizeof pointer 'sizeof(P*)/sizeof(Q*)'
   sum += sizeof(ptr) / sizeof(void*);
-  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of sizeof pointer 'sizeof(P*)/sizeof(Q*)'
+  // CHECK-MESSAGES: :[[@LINE-1]]:22: warning: suspicious usage of sizeof pointer 'sizeof(P*)/sizeof(Q*)'
   sum += sizeof(ptr) / sizeof(const void volatile*);
-  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of sizeof pointer 'sizeof(P*)/sizeof(Q*)'
+  // CHECK-MESSAGES: :[[@LINE-1]]:22: warning: suspicious usage of sizeof pointer 'sizeof(P*)/sizeof(Q*)'
   sum += sizeof(ptr) / sizeof(char);
-  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of sizeof pointer 'sizeof(T*)/sizeof(T)'
+  // CHECK-MESSAGES: :[[@LINE-1]]:22: warning: suspicious usage of sizeof pointer 'sizeof(T*)/sizeof(T)'
   sum += sizeof(ptr) / sizeof(ptr[0]);
-  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of sizeof pointer 'sizeof(T*)/sizeof(T)'
+  // CHECK-MESSAGES: :[[@LINE-1]]:22: warning: suspicious usage of sizeof pointer 'sizeof(T*)/sizeof(T)'
   sum += sizeof(int) * sizeof(char);
-  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious 'sizeof' by 'sizeof' multiplication
+  // CHECK-MESSAGES: 

[PATCH] D101617: [clang-tidy] Tweak diag ranges for bugprone-sizeof-expression

[Piotr Zegar via Phabricator via cfe-commits]

PiotrZSL accepted this revision.
PiotrZSL added a comment.
This revision is now accepted and ready to land.
Herald added a project: All.

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101617

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

[PATCH] D119165: [clang-tidy] Add processing lambda captures at bugprone-use-after-move check

[Piotr Zegar via Phabricator via cfe-commits]

PiotrZSL abandoned this revision.
PiotrZSL added a comment.

Already fixed by D126780 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119165

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

[PATCH] D115106: [clang-tidy] Fix `readability-static-accessed-through-instance` false negative for static methods

[Piotr Zegar via Phabricator via cfe-commits]

PiotrZSL abandoned this revision.
PiotrZSL added a comment.

Already fixed by D157326 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115106

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

[PATCH] D130639: [clang-tidy] Fix readability-redundant-string-c-str fix for overloaded operator->

[Piotr Zegar via Phabricator via cfe-commits]

PiotrZSL abandoned this revision.
PiotrZSL added a comment.

Already fixed by D145730 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130639

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

[PATCH] D117460: [clang-tidy][NFC] Reduce map lookups in IncludeSorter

[Piotr Zegar via Phabricator via cfe-commits]

PiotrZSL added a comment.

This change is dangerous, it depends that elements/iterators won't be 
re-allocated/invalidated during insertion process.
Part with removing double lookup in IncludeSorter::addInclude looks fine, but 
part that involve IncludeBucket is problematic.
I will try to push part that involve IncludeLocations optimizations.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117460

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

[PATCH] D125949: [clang-tidy] modernize-avoid-bind: Fix crash when method name is not a simple identifier

[Piotr Zegar via Phabricator via cfe-commits]

PiotrZSL updated this revision to Diff 558074.
PiotrZSL added a comment.

Rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125949

Files:
  clang-tools-extra/clang-tidy/modernize/AvoidBindCheck.cpp
  clang-tools-extra/test/clang-tidy/checkers/modernize/avoid-bind.cpp


Index: clang-tools-extra/test/clang-tidy/checkers/modernize/avoid-bind.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/modernize/avoid-bind.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/modernize/avoid-bind.cpp
@@ -43,6 +43,7 @@
 struct D {
   D() = default;
   void operator()(int x, int y) const {}
+  operator bool() const { return true; }
 
   void MemberFunction(int x) {}
   int MemberFunctionWithReturn(int x) {}
@@ -342,6 +343,7 @@
 struct E {
   void MemberFunction(int x) {}
   int MemberFunctionWithReturn(int x) {}
+  int operator()(int x, int y) const { return x + y; }
 
   void testMemberFunctions() {
 D *d;
@@ -379,6 +381,26 @@
 auto HHH = std::bind(::MemberFunctionWithReturn, _1, 1);
 // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: prefer a lambda to std::bind
 // CHECK-FIXES: auto HHH = [](auto && PH1) { return 
PH1->MemberFunctionWithReturn(1); };
+
+auto III = std::bind(::operator(), d, 1, 2);
+// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: prefer a lambda to std::bind
+// CHECK-FIXES: auto III = [d] { (*d)(1, 2); }
+
+auto JJJ = std::bind(::operator(), , 1, 2);
+// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: prefer a lambda to std::bind
+// CHECK-FIXES: auto JJJ = [ObjectPtr = ] { (*ObjectPtr)(1, 2); }
+
+auto KKK = std::bind(::operator(), _1, 1, 2);
+// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: prefer a lambda to std::bind
+// CHECK-FIXES: auto KKK = [](auto && PH1) { (*PH1)(1, 2); };
+
+auto LLL = std::bind(::operator bool, d);
+// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: prefer a lambda to std::bind
+// CHECK-FIXES: auto LLL = [d] { return d->operator bool(); }
+
+auto MMM = std::bind(::operator(), this, 1, 2);
+// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: prefer a lambda to std::bind
+// CHECK-FIXES: auto MMM = [this] { return (*this)(1, 2); }
   }
 };
 
Index: clang-tools-extra/clang-tidy/modernize/AvoidBindCheck.cpp
===
--- clang-tools-extra/clang-tidy/modernize/AvoidBindCheck.cpp
+++ clang-tools-extra/clang-tidy/modernize/AvoidBindCheck.cpp
@@ -691,12 +691,15 @@
 const auto *MethodDecl = dyn_cast(LP.Callable.Decl);
 const BindArgument  = FunctionCallArgs.front();
 
-if (!isa(ignoreTemporariesAndPointers(ObjPtr.E))) {
-  Stream << ObjPtr.UsageIdentifier;
-  Stream << "->";
+if (MethodDecl->getOverloadedOperator() == OO_Call) {
+  Stream << "(*" << ObjPtr.UsageIdentifier << ')';
+} else {
+  if (!isa(ignoreTemporariesAndPointers(ObjPtr.E))) {
+Stream << ObjPtr.UsageIdentifier;
+Stream << "->";
+  }
+  Stream << MethodDecl->getNameAsString();
 }
-
-Stream << MethodDecl->getName();
   } else {
 switch (LP.Callable.CE) {
 case CE_Var:


Index: clang-tools-extra/test/clang-tidy/checkers/modernize/avoid-bind.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/modernize/avoid-bind.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/modernize/avoid-bind.cpp
@@ -43,6 +43,7 @@
 struct D {
   D() = default;
   void operator()(int x, int y) const {}
+  operator bool() const { return true; }
 
   void MemberFunction(int x) {}
   int MemberFunctionWithReturn(int x) {}
@@ -342,6 +343,7 @@
 struct E {
   void MemberFunction(int x) {}
   int MemberFunctionWithReturn(int x) {}
+  int operator()(int x, int y) const { return x + y; }
 
   void testMemberFunctions() {
 D *d;
@@ -379,6 +381,26 @@
 auto HHH = std::bind(::MemberFunctionWithReturn, _1, 1);
 // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: prefer a lambda to std::bind
 // CHECK-FIXES: auto HHH = [](auto && PH1) { return PH1->MemberFunctionWithReturn(1); };
+
+auto III = std::bind(::operator(), d, 1, 2);
+// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: prefer a lambda to std::bind
+// CHECK-FIXES: auto III = [d] { (*d)(1, 2); }
+
+auto JJJ = std::bind(::operator(), , 1, 2);
+// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: prefer a lambda to std::bind
+// CHECK-FIXES: auto JJJ = [ObjectPtr = ] { (*ObjectPtr)(1, 2); }
+
+auto KKK = std::bind(::operator(), _1, 1, 2);
+// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: prefer a lambda to std::bind
+// CHECK-FIXES: auto KKK = [](auto && PH1) { (*PH1)(1, 2); };
+
+auto LLL = std::bind(::operator bool, d);
+// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: prefer a lambda to std::bind
+// CHECK-FIXES: auto LLL = [d] { return d->operator bool(); }
+
+auto 

[PATCH] D125949: [clang-tidy] modernize-avoid-bind: Fix crash when method name is not a simple identifier

[Piotr Zegar via Phabricator via cfe-commits]

PiotrZSL accepted this revision.
PiotrZSL added a comment.
This revision is now accepted and ready to land.
Herald added a reviewer: njames93.

Crash no longer happens on main, but this change introduce nice fix for 
operators handling.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125949

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

[PATCH] D138846: MC/DC in LLVM Source-Based Code Coverage: LLVM back-end and compiler-rt

[Hans Wennborg via Phabricator via cfe-commits]

hans added a comment.

In D138846#4656594 , @hans wrote:

> Just a heads up that we're seeing crashes in the Rust compiler which bisects 
> to this change. Investigating in 
> https://bugs.chromium.org/p/chromium/issues/detail?id=1500558

Here's a small reproducer:

  $ cat /tmp/x.ll
  target triple = "x86_64-unknown-linux-gnu"
  
  declare void @llvm.instrprof.increment(ptr %0, i64 %1, i32 %2, i32 %3)
  @prof_name = private constant [9 x i8] c"prof_name"
  
  define internal void @f() {
call void @llvm.instrprof.increment(ptr @prof_name, i64 123456, i32 32, i32 
0)
ret void
  }
  
  define void @foo() {
call void @f()
ret void
  }
  
  define void @bar() {
call void @f()
ret void
  }
  
  $ build/bin/opt /tmp/x.ll -passes='cgscc(inline),instrprof' -S
  PLEASE submit a bug report to https://github.com/llvm/llvm-project/issues/ 
and include the crash backtrace.
  Stack dump:
  0.  Program arguments: build/bin/opt /tmp/x.ll 
-passes=cgscc(inline),instrprof -S
   #0 0x55fcf4193d48 llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) 
(build/bin/opt+0x2f77d48)
   #1 0x55fcf419186e llvm::sys::RunSignalHandlers() 
(build/bin/opt+0x2f7586e)
   #2 0x55fcf41943fd SignalHandler(int) Signals.cpp:0:0
   #3 0x7f0cbe05a510 (/lib/x86_64-linux-gnu/libc.so.6+0x3c510)
   #4 0x55fcf3b279c9 llvm::GlobalVariable::eraseFromParent() 
(build/bin/opt+0x290b9c9)
   #5 0x55fcf47a87c8 llvm::InstrProfiling::emitNameData() 
(build/bin/opt+0x358c7c8)
   #6 0x55fcf47a32e3 llvm::InstrProfiling::run(llvm::Module&, 
std::function) 
(build/bin/opt+0x35872e3)
   #7 0x55fcf47a2b99 llvm::InstrProfiling::run(llvm::Module&, 
llvm::AnalysisManager&) (build/bin/opt+0x3586b99)
   #8 0x55fcf43a31ed llvm::detail::PassModel>::run(llvm::Module&, 
llvm::AnalysisManager&) (build/bin/opt+0x31871ed)
   #9 0x55fcf3bacf04 llvm::PassManager>::run(llvm::Module&, 
llvm::AnalysisManager&) (build/bin/opt+0x2990f04)
  #10 0x55fcf1b479db llvm::runPassPipeline(llvm::StringRef, llvm::Module&, 
llvm::TargetMachine*, llvm::TargetLibraryInfoImpl*, llvm::ToolOutputFile*, 
llvm::ToolOutputFile*, llvm::ToolOutputFile*, llvm::StringRef, 
llvm::ArrayRef, llvm::opt_tool::OutputKind, 
llvm::opt_tool::VerifierKind, bool, bool, bool, bool, bool, bool, bool) 
(build/bin/opt+0x92b9db)
  #11 0x55fcf1b54e95 main (build/bin/opt+0x938e95)
  #12 0x7f0cbe0456ca __libc_start_call_main 
./csu/../sysdeps/nptl/libc_start_call_main.h:74:3
  #13 0x7f0cbe045785 call_init ./csu/../csu/libc-start.c:128:20
  #14 0x7f0cbe045785 __libc_start_main ./csu/../csu/libc-start.c:347:5
  #15 0x55fcf1b4156a _start (build/bin/opt+0x92556a)
  Segmentation fault


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138846

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

[PATCH] D138846: MC/DC in LLVM Source-Based Code Coverage: LLVM back-end and compiler-rt

[Mike Hommey via Phabricator via cfe-commits]

glandium added a comment.

Code built with older versions of LLVM (e.g. rust) and running with the updated 
runtime crash at startup with this change.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138846

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

[PATCH] D138846: MC/DC in LLVM Source-Based Code Coverage: LLVM back-end and compiler-rt

[Hans Wennborg via Phabricator via cfe-commits]

hans added a comment.

Just a heads up that we're seeing crashes in the Rust compiler which bisects to 
this change. Investigating in 
https://bugs.chromium.org/p/chromium/issues/detail?id=1500558


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138846

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

[PATCH] D147073: [Coverage] Handle invalid end location of an expression/statement.

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added inline comments.



Comment at: clang/lib/CodeGen/CoverageMappingGen.cpp:607
+// compiler has broken.
+assert((!StartLoc || StartLoc->isValid()) && "Start location is not 
valid");
+assert((!EndLoc || EndLoc->isValid()) && "End location is not valid");

Is this workaround still needed after b0e61de7075942ef5ac8af9ca1ec918317f62152 
(with a test `clang/test/Coverage/unresolved-ctor-expr.cpp`)?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147073

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

[PATCH] D155529: [clang-format] Add SpaceInParensOption for __attribute__ keyword

[Owen Pan via Phabricator via cfe-commits]

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

In D155529#4509686 , @MyDeveloperDay 
wrote:

> We should never make assumptions about what people do/don't use



> If you have this you have to honour it.. if 'SpacesInParentheses ' was true 
> then 'InAttributeSpecifiers' needs to be true by default, shouldn't there be 
> something in the expanding out of SpacesInParentheses

Because `SpacesInParentheses` was added 10+ years ago in rGb55acad91c37 
 and 
`__attribute__((foo))` was not in the unit tests, it's probably a bug that the 
double parens were not excluded. Not sure whether the users who didn't complain 
about it really wanted it or simply didn't bother. The only way to find out 
would be to fix the bug, assuming it indeed was a bug.

When fixing bugs, especially the very old ones, we often have to choose between 
just fixing the bugs and adding an option to avoid "regressions", and I usually 
prefer the former. Nevertheless, I would have no problem if this new option is 
extended to handle all double parens, e.g. `if (( i = j ))`, `decltype(( x ))`, 
etc.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155529

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

[PATCH] D156360: [clang-format] Support function and overloaded operator SpacesInParensOption

[Owen Pan via Phabricator via cfe-commits]

owenpan added inline comments.



Comment at: clang/lib/Format/TokenAnnotator.cpp:3970-4009
   if (Left.is(tok::l_paren) || Right.is(tok::r_paren)) {
 if (Right.is(TT_CastRParen) ||
 (Left.MatchingParen && Left.MatchingParen->is(TT_CastRParen))) {
   return Style.SpacesInParensOptions.InCStyleCasts;
 }
 if (Left.isOneOf(TT_AttributeLParen, TT_AttributeRParen) ||
 Right.isOneOf(TT_AttributeLParen, TT_AttributeRParen) ||

Consider wrapping this in a function or lambda. Not sure if it would simply the 
logic with the parens being handled separately:
```
if (Left.is(tok::l_paren)) {
  ...
} else if (Right.is(tok::r_paren)) {
  ...
}



Comment at: clang/lib/Format/TokenAnnotator.cpp:3990
+  return Style.SpacesInParensOptions.InFunctionDefinitions;
+else
+  return Style.SpacesInParensOptions.InFunctionDeclarations;

No `else` after `return`.



Comment at: clang/unittests/Format/FormatTest.cpp:16786
   verifyFormat("SomeType *__attribute__( ( attr ) ) *a = NULL;", Spaces);
-  verifyFormat("void __attribute__( ( naked ) ) foo( int bar )", Spaces);
+  verifyFormat("void __attribute__( ( x ) ) foo(int y) { return; }", Spaces);
   verifyFormat("void f() __attribute__( ( asdf ) );", Spaces);

gedare wrote:
> HazardyKnusperkeks wrote:
> > Why change this?
> The original test is incomplete/ambiguous. It's either a declaration missing 
> a semicolon, or it's the start of a definition. I made it a definition.
Then can you also add a declaration?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156360

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

[PATCH] D154755: [clang-format] Fix formatting of if statements with BlockIndent

[Gedare Bloom via Phabricator via cfe-commits]

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



Comment at: clang/lib/Format/ContinuationIndenter.cpp:773-792
   if ((Style.AlignAfterOpenBracket == FormatStyle::BAS_AlwaysBreak ||
Style.AlignAfterOpenBracket == FormatStyle::BAS_BlockIndent) &&
   (Previous.isOneOf(tok::l_paren, TT_TemplateOpener, tok::l_square) ||
(Previous.is(tok::l_brace) && Previous.isNot(BK_Block) &&
 Style.Cpp11BracedListStyle)) &&
   State.Column > getNewLineColumn(State) &&
   (!Previous.Previous ||

owenpan wrote:
> This is getting out of hand. Consider writing a lambda with early returns.
I would probably not get to refactoring this until January.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154755

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

[PATCH] D154755: [clang-format] Fix formatting of if statements with BlockIndent

[Gedare Bloom via Phabricator via cfe-commits]

gedare updated this revision to Diff 558069.
gedare added a comment.

Use cameLCAse in test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154755

Files:
  clang/lib/Format/ContinuationIndenter.cpp
  clang/unittests/Format/FormatTest.cpp


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -25789,8 +25789,8 @@
"}",
Style);
 
-  verifyFormat("if (quitelongarg !=\n"
-   "(alsolongarg - 1)) { // ABC is a very long "
+  verifyFormat("if (quiteLongArg !=\n"
+   "(alsoLongArg - 1)) { // ABC is a very long "
"comment\n"
"  return;\n"
"}",
@@ -25803,12 +25803,44 @@
"}",
Style);
 
-  verifyFormat("if (quitelongarg !=\n"
-   "(alsolongarg - 1)) { // ABC is a very long "
+  verifyFormat("if (quiteLongArg !=\n"
+   "(alsoLongArg - 1)) { // ABC is a very long "
"comment\n"
"  return;\n"
"}",
Style);
+
+  verifyFormat("void foo() {\n"
+   "  if (camelCaseName < alsoLongName ||\n"
+   "  anotherEvenLongerName <=\n"
+   "  thisReallyReallyReallyReallyReallyReallyLongerName 
||"
+   "\n"
+   "  otherName < thisLastName) {\n"
+   "return;\n"
+   "  } else if (quiteLongName < alsoLongName ||\n"
+   " anotherEvenLongerName <=\n"
+   " 
thisReallyReallyReallyReallyReallyReallyLonger"
+   "Name ||\n"
+   " otherName < thisLastName) {\n"
+   "return;\n"
+   "  }\n"
+   "}",
+   Style);
+
+  Style.ContinuationIndentWidth = 2;
+  verifyFormat("void foo() {\n"
+   "  if (ThisIsRatherALongIfClause && thatIExpectToBeBroken ||\n"
+   "  ontoMultipleLines && whenFormattedCorrectly) {\n"
+   "if (false) {\n"
+   "  return;\n"
+   "} else if (thisIsRatherALongIfClause && "
+   "thatIExpectToBeBroken ||\n"
+   "   ontoMultipleLines && whenFormattedCorrectly) 
{\n"
+   "  return;\n"
+   "}\n"
+   "  }\n"
+   "}",
+   Style);
 }
 
 TEST_F(FormatTest, AlignAfterOpenBracketBlockIndentForStatement) {
Index: clang/lib/Format/ContinuationIndenter.cpp
===
--- clang/lib/Format/ContinuationIndenter.cpp
+++ clang/lib/Format/ContinuationIndenter.cpp
@@ -777,8 +777,10 @@
 Style.Cpp11BracedListStyle)) &&
   State.Column > getNewLineColumn(State) &&
   (!Previous.Previous ||
-   !Previous.Previous->isOneOf(TT_CastRParen, tok::kw_for, tok::kw_while,
-   tok::kw_switch)) &&
+   !(Previous.Previous->isOneOf(TT_CastRParen, tok::kw_for, tok::kw_while,
+tok::kw_switch) ||
+ (Style.AlignAfterOpenBracket == FormatStyle::BAS_BlockIndent &&
+  Previous.Previous->isIf( &&
   // Don't do this for simple (no expressions) one-argument function calls
   // as that feels like needlessly wasting whitespace, e.g.:
   //


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -25789,8 +25789,8 @@
"}",
Style);
 
-  verifyFormat("if (quitelongarg !=\n"
-   "(alsolongarg - 1)) { // ABC is a very long "
+  verifyFormat("if (quiteLongArg !=\n"
+   "(alsoLongArg - 1)) { // ABC is a very long "
"comment\n"
"  return;\n"
"}",
@@ -25803,12 +25803,44 @@
"}",
Style);
 
-  verifyFormat("if (quitelongarg !=\n"
-   "(alsolongarg - 1)) { // ABC is a very long "
+  verifyFormat("if (quiteLongArg !=\n"
+   "(alsoLongArg - 1)) { // ABC is a very long "
"comment\n"
"  return;\n"
"}",
Style);
+
+  verifyFormat("void foo() {\n"
+   "  if (camelCaseName < alsoLongName ||\n"
+   "  anotherEvenLongerName <=\n"
+   "  thisReallyReallyReallyReallyReallyReallyLongerName ||"
+   "\n"
+   "  otherName < thisLastName) {\n"
+   "return;\n"
+   "  } else if 

[PATCH] D154755: [clang-format] Fix formatting of if statements with BlockIndent

[Owen Pan via Phabricator via cfe-commits]

owenpan added inline comments.



Comment at: clang/unittests/Format/FormatTest.cpp:25814-25824
+   "  if (quitelongname < alsolongname ||\n"
+   "  anotherevenlongername <=\n"
+   "  thisreallyreallyreallyreallyreallyreallylongername 
||"
+   "\n"
+   "  othername < thislastname) {\n"
+   "return;\n"
+   "  } else if (quitelongname < alsolongname ||\n"

Please use camel case.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154755

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

[PATCH] D154755: [clang-format] Fix formatting of if statements with BlockIndent

[Owen Pan via Phabricator via cfe-commits]

owenpan added inline comments.



Comment at: clang/lib/Format/ContinuationIndenter.cpp:773-792
   if ((Style.AlignAfterOpenBracket == FormatStyle::BAS_AlwaysBreak ||
Style.AlignAfterOpenBracket == FormatStyle::BAS_BlockIndent) &&
   (Previous.isOneOf(tok::l_paren, TT_TemplateOpener, tok::l_square) ||
(Previous.is(tok::l_brace) && Previous.isNot(BK_Block) &&
 Style.Cpp11BracedListStyle)) &&
   State.Column > getNewLineColumn(State) &&
   (!Previous.Previous ||

This is getting out of hand. Consider writing a lambda with early returns.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154755

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