[PATCH] D122830: [clang][dataflow] Add support for (built-in) (in)equality operators

[Stanislav Gatev via Phabricator via cfe-commits]

sgatev accepted this revision.
sgatev added inline comments.



Comment at: clang/lib/Analysis/FlowSensitive/Transfer.cpp:53
+  if (auto *LHSValue = dyn_cast_or_null(
+  Env.getValue(*LHSNorm, SkipPast::Reference)))
+if (auto *RHSValue = dyn_cast_or_null(

Do we need to skip past references here? If so, then let's add a test that 
fails if these are changed to `SkipPast::None`.



Comment at: clang/lib/Analysis/FlowSensitive/Transfer.cpp:109-113
+  auto  = evaluateBooleanEquality(*LHS, *RHS, Env);
+  auto  = Env.createStorageLocation(*S);
+  Env.setStorageLocation(*S, Loc);
+  Env.setValue(Loc, S->getOpcode() == BO_EQ ? LHSAndRHSValue
+: Env.makeNot(LHSAndRHSValue));





Comment at: clang/unittests/Analysis/FlowSensitive/TransferTest.cpp:2413
 
+TEST_F(TransferTest, Equality) {
+  std::string Code = R"(





Comment at: clang/unittests/Analysis/FlowSensitive/TransferTest.cpp:2448
+
+TEST_F(TransferTest, Inequality) {
+  std::string Code = R"(




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122830

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

[PATCH] D122273: [clang][dataflow] Fix handling of base-class fields

[Stanislav Gatev via Phabricator via cfe-commits]

sgatev accepted this revision.
sgatev added inline comments.



Comment at: clang/include/clang/Analysis/FlowSensitive/StorageLocation.h:59-60
 /// can be traced independently by abstract interpretation. For example: a
-/// struct with public members.
+/// struct with public members. Note that the corresponding `StructValue` has a
+/// flat layout that does not contain the child locations stored here.
 class AggregateStorageLocation final : public StorageLocation {

I find this a bit confusing. `StructValue` does not contain storage locations 
in general. I think we should make it clear that the layout of 
`AggregateStorageLocation` is flat, i.e. if it's used for a `struct` or `class` 
type it will contain child storage locations for all accessible members of base 
`struct` and `class` types.



Comment at: clang/include/clang/Analysis/FlowSensitive/Value.h:193-194
+/// Models a value of `struct` or `class` type. Implements a flat struct 
layout,
+/// where the child values are directly reachable from the struct value (rather
+/// than indirectly, through `StorageLocation`s).
 class StructValue final : public Value {

I'm not sure what's meant here by indirect reachability, but I suggest 
documenting that a `StructValue` that models a `struct` or `class` type 
contains child values for all accessible members of its base `struct` and 
`class` types. 



Comment at: clang/unittests/Analysis/FlowSensitive/TransferTest.cpp:1097-1100
+const auto *FooLoc = cast(
+Env.getStorageLocation(*FooDecl, SkipPast::None));
+const auto  = *cast(Env.getValue(*FooLoc));
+EXPECT_THAT(FooVal.getChild(*APublicDecl), NotNull());





Comment at: clang/unittests/Analysis/FlowSensitive/TransferTest.cpp:1102-
+// Base-class fields.
+EXPECT_THAT(FooVal.getChild(*ADefaultDecl), IsNull());
+EXPECT_THAT(FooVal.getChild(*APrivateDecl), IsNull());
+EXPECT_THAT(FooVal.getChild(*AProtectedDecl), NotNull());
+EXPECT_THAT(FooVal.getChild(*APublicDecl), NotNull());
+
+// Derived-class fields.

Let's also check `FooLoc`'s child storage locations. Same for the test below.



Comment at: clang/unittests/Analysis/FlowSensitive/TransferTest.cpp:1156-1159
+const auto *FooLoc = cast(
+Env.getStorageLocation(*FooDecl, SkipPast::None));
+const auto *FooVal = cast(Env.getValue(*FooLoc));
+EXPECT_THAT(FooVal->getChild(*BarDecl), NotNull());




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122273

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

[PATCH] D122699: [HLSL] Add Semantic syntax, and SV_GroupIndex

[Xiang Li via Phabricator via cfe-commits]

python3kgae added inline comments.



Comment at: clang/lib/Sema/SemaDeclAttr.cpp:6916
+  Triple Target = S.Context.getTargetInfo().getTriple();
+  if (Target.getEnvironment() != Triple::Compute) {
+uint32_t Pipeline =

It is OK to have SV_GroupIndex on a compute shader entry for library profile.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122699

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

[PATCH] D122865: [HLSL][clang][Driver] Support target profile command line option.

[Xiang Li via Phabricator via cfe-commits]

python3kgae created this revision.
python3kgae added reviewers: beanz, steven_wu, JonChesterfield.
python3kgae added a project: clang.
Herald added a subscriber: mgorny.
Herald added a reviewer: sscalpone.
Herald added a project: All.
python3kgae requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay.

The target profile option(/T) decide the shader model when compile hlsl.
The format is shaderKind_major_minor like ps_6_1.
The shader model is saved as llvm::Triple is clang/llvm like 
dxil-unknown-shadermodel6.1-hull.
The main job to support the option is translating ps_6_1 into 
shadermodel6.1-pixel.
That is done inside tryParseProfile  at HLSL.cpp.

To integrate the option into clang Driver, a new DriverMode DxcMode is created. 
When DxcMode is enabled, OSType for TargetTriple will be forced into 
Triple::ShaderModel. And new ToolChain HLSLToolChain will be created when 
OSType is Triple::ShaderModel.

In HLSLToolChain, ComputeEffectiveClangTriple is overridden to call 
tryParseProfile when targetProfile option is set.

To make test work, Fo option is added and .hlsl is added for active -xhlsl.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D122865

Files:
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/include/clang/Driver/Driver.h
  clang/include/clang/Driver/Options.h
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/CMakeLists.txt
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChain.cpp
  clang/lib/Driver/ToolChains/HLSL.cpp
  clang/lib/Driver/ToolChains/HLSL.h
  clang/lib/Driver/Types.cpp
  clang/test/Driver/dxil_target_profile.hlsl
  clang/test/lit.cfg.py
  clang/unittests/Driver/ToolChainTest.cpp

Index: clang/unittests/Driver/ToolChainTest.cpp
===
--- clang/unittests/Driver/ToolChainTest.cpp
+++ clang/unittests/Driver/ToolChainTest.cpp
@@ -300,6 +300,12 @@
   EXPECT_TRUE(Res.ModeSuffix == "clang-cl");
   EXPECT_STREQ(Res.DriverMode, "--driver-mode=cl");
   EXPECT_FALSE(Res.TargetIsValid);
+
+  Res = ToolChain::getTargetAndModeFromProgramName("clang-dxc");
+  EXPECT_TRUE(Res.TargetPrefix.empty());
+  EXPECT_TRUE(Res.ModeSuffix == "clang-dxc");
+  EXPECT_TRUE(Res.DriverMode == "--driver-mode=dxc");
+  EXPECT_FALSE(Res.TargetIsValid);
 }
 
 TEST(ToolChainTest, CommandOutput) {
Index: clang/test/lit.cfg.py
===
--- clang/test/lit.cfg.py
+++ clang/test/lit.cfg.py
@@ -67,6 +67,8 @@
 'clang-tblgen', 'clang-scan-deps', 'opt', 'llvm-ifs', 'yaml2obj',
 ToolSubst('%clang_extdef_map', command=FindTool(
 'clang-extdef-mapping'), unresolved='ignore'),
+ToolSubst('%clang_dxc', command=config.clang,
+extra_args=['--driver-mode=dxc']),
 ]
 
 if config.clang_examples:
Index: clang/test/Driver/dxil_target_profile.hlsl
===
--- /dev/null
+++ clang/test/Driver/dxil_target_profile.hlsl
@@ -0,0 +1,35 @@
+// RUN: %clang_dxc -Tvs_6_0 -Fo - %s 2>&1 | FileCheck %s --check-prefix=VS60
+// VS60:target triple = "dxil-unknown-shadermodel6.0-vertex"
+
+// RUN: %clang_dxc -Ths_6_1  -Fo - %s 2>&1 | FileCheck %s --check-prefix=HS61
+// HS61:target triple = "dxil-unknown-shadermodel6.1-hull"
+
+// RUN: %clang_dxc -Tds_6_2  -Fo - %s 2>&1 | FileCheck %s --check-prefix=DS62
+// DS62:target triple = "dxil-unknown-shadermodel6.2-domain"
+
+// RUN: %clang_dxc -Tgs_6_3 -Fo - %s 2>&1 | FileCheck %s --check-prefix=GS63
+// GS63:target triple = "dxil-unknown-shadermodel6.3-geometry"
+
+// RUN: %clang_dxc -Tps_6_4 -Fo - %s 2>&1 | FileCheck %s --check-prefix=PS64
+// PS64:target triple = "dxil-unknown-shadermodel6.4-pixel"
+
+// RUN: %clang_dxc -Tms_6_5 -Fo - %s 2>&1 | FileCheck %s --check-prefix=MS65
+// MS65:target triple = "dxil-unknown-shadermodel6.5-mesh"
+
+// RUN: %clang_dxc -Tas_6_6 -Fo - %s 2>&1 | FileCheck %s --check-prefix=AS66
+// AS66:target triple = "dxil-unknown-shadermodel6.6-amplification"
+
+// RUN: %clang_dxc -Tlib_6_x  -Fo - %s 2>&1 | FileCheck %s --check-prefix=LIB6x
+// LIB6x:target triple = "dxil-unknown-shadermodel6.15-library"
+
+// RUN: %clang_dxc -###  -Tps_3_1  -Fo - %s 2>&1 | FileCheck %s --check-prefix=INVALID
+// INVALID:invalid profile : ps_3_1
+
+// RUN: %clang_dxc -###  -Tlib_6_1  -Fo - %s 2>&1 | FileCheck %s --check-prefix=INVALID2
+// INVALID2:invalid profile : lib_6_1
+
+// RUN: %clang_dxc -###  -Tms_6_1  -Fo - %s 2>&1 | FileCheck %s --check-prefix=INVALID3
+// INVALID3:invalid profile : ms_6_1
+
+// RUN: %clang_dxc -###  -Tas_6_4  -Fo - %s 2>&1 | FileCheck %s --check-prefix=INVALID4
+// INVALID4:invalid profile : as_6_4
Index: clang/lib/Driver/Types.cpp
===
--- clang/lib/Driver/Types.cpp
+++ clang/lib/Driver/Types.cpp
@@ -332,6 +332,7 @@
.Case("c++m", TY_CXXModule)
.Case("cppm", TY_CXXModule)
.Case("cxxm", TY_CXXModule)
+ 

[PATCH] D117835: [OpenMPIRBuilder] Detect and fix ambiguous InsertPoints for createSections.

[Shraiysh via Phabricator via cfe-commits]

shraiysh added inline comments.



Comment at: 
mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp:74-75
+  // for the allocs.
+  // TODO: Create a dedicated alloca BasicBlock at function creation such that
+  // we do not need to move the current InertPoint here.
+  if (builder.GetInsertBlock() ==

[Suggestion] We probably don't need to do this, because if a conversion does 
not require an alloca (for example, barrier), we will be creating a basicblock 
unnecessarily. So, creating this on-demand seems okay to me.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117835

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

[PATCH] D117835: [OpenMPIRBuilder] Detect and fix ambiguous InsertPoints for createSections.

[Shraiysh via Phabricator via cfe-commits]

shraiysh added a comment.

LGTM. Can we please rebase this and fix the same in `convertOmpAtomicUpdate` 
and `convertOmpAtomicCapture` too?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117835

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

[PATCH] D122377: [PowerPC] Support 16-byte lock free atomics on pwr8 and up

[Kai Luo via Phabricator via cfe-commits]

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



Comment at: clang/lib/Basic/Targets/PPC.h:448
+  void setMaxAtomicWidth() override {
+// For layout on ELF targets, we support up to 16 bytes.
+if (getTriple().isOSBinFormatELF())

hubert.reinterpretcast wrote:
> I believe this should be presented more as this override being implemented 
> currently only for ELF targets. The current presentation seems to imply more 
> design intent for non-ELF targets than there is consensus for.
> 
> For example:
> ```
> if (!getTriple().isOSBinFormatELF())
>   return PPCTargetInfo::setMaxAtomicWidth();
> ```
It looks a chance to adjust according to arch level to me(Considering we 
finally will make xcoff and elf targets consistent here). If you have strong 
opinion on this, I'll make a change here.



Comment at: clang/test/CodeGen/PowerPC/atomic-alignment.c:34
 
 // PPC32: @o = global %struct.O zeroinitializer, align 1{{$}}
 // PPC64: @o = global %struct.O zeroinitializer, align 8{{$}}

hubert.reinterpretcast wrote:
> Just noting that GCC increases the alignment even for ppc32:
> ```
> typedef struct A8 { char x[8]; } A8;
> typedef struct A16 { char x[16]; } A16;
> extern char q8[_Alignof(_Atomic(A8))], q8[8]; // okay for GCC targeting ppc32
> extern char q16[_Alignof(_Atomic(A16))], q16[16]; // okay for GCC targeting 
> ppc32
> ```
> 
> Apparently, the change for i686 in GCC occurred with version 11.
> https://godbolt.org/z/fTTGoqWW1
I'll post another patch to address ppc32 issue.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122377

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

[PATCH] D122377: [PowerPC] Support 16-byte lock free atomics on pwr8 and up

[Kai Luo via Phabricator via cfe-commits]

lkail updated this revision to Diff 419618.
lkail retitled this revision from "[PowerPC][Linux] Support 16-byte lock free 
atomics on pwr8 and up" to "[PowerPC] Support 16-byte lock free atomics on pwr8 
and up".
lkail edited the summary of this revision.
lkail added a comment.

Make 16-byte atomic type aligned to 16-byte on PPC64.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122377

Files:
  clang/lib/Basic/Targets/PPC.cpp
  clang/lib/Basic/Targets/PPC.h
  clang/test/CodeGen/PowerPC/atomic-alignment.c
  clang/test/CodeGen/PowerPC/quadword-atomics.c
  clang/test/Sema/atomic-ops.c
  llvm/lib/Target/PowerPC/PPCISelLowering.cpp
  llvm/lib/Target/PowerPC/PPCISelLowering.h
  llvm/test/CodeGen/PowerPC/atomics-i128.ll

Index: llvm/test/CodeGen/PowerPC/atomics-i128.ll
===
--- llvm/test/CodeGen/PowerPC/atomics-i128.ll
+++ llvm/test/CodeGen/PowerPC/atomics-i128.ll
@@ -5,6 +5,18 @@
 ; RUN: llc -verify-machineinstrs -mtriple=powerpc64-unknown-unknown -mcpu=pwr7 \
 ; RUN:   -ppc-asm-full-reg-names -ppc-quadword-atomics \
 ; RUN:   -ppc-track-subreg-liveness < %s | FileCheck --check-prefix=PWR7 %s
+; RUN: llc -verify-machineinstrs -mtriple=powerpc64le-unknown-linux-gnu -mcpu=pwr8 \
+; RUN:   -ppc-asm-full-reg-names -ppc-track-subreg-liveness < %s | FileCheck \
+; RUN:   --check-prefix=LE-PWR8 %s
+; RUN: llc -verify-machineinstrs -mtriple=powerpc64le-unknown-freebsd -mcpu=pwr8 \
+; RUN:   -ppc-asm-full-reg-names -ppc-track-subreg-liveness < %s | FileCheck \
+; RUN:   --check-prefix=LE-PWR8 %s
+; RUN: llc -verify-machineinstrs -mtriple=powerpc64-unknown-aix -mcpu=pwr8 \
+; RUN:   -ppc-asm-full-reg-names -ppc-track-subreg-liveness < %s | FileCheck \
+; RUN:   --check-prefix=AIX64-PWR8 %s
+; RUN: llc -verify-machineinstrs -mtriple=powerpc-unknown-unknown -mcpu=pwr8 \
+; RUN:   -ppc-quadword-atomics -ppc-asm-full-reg-names -ppc-track-subreg-liveness < %s \
+; RUN: | FileCheck --check-prefix=AIX32-PWR8 %s
 
 
 define i128 @swap(i128* %a, i128 %x) {
@@ -39,6 +51,62 @@
 ; PWR7-NEXT:ld r0, 16(r1)
 ; PWR7-NEXT:mtlr r0
 ; PWR7-NEXT:blr
+;
+; LE-PWR8-LABEL: swap:
+; LE-PWR8:   # %bb.0: # %entry
+; LE-PWR8-NEXT:sync
+; LE-PWR8-NEXT:  .LBB0_1: # %entry
+; LE-PWR8-NEXT:#
+; LE-PWR8-NEXT:lqarx r6, 0, r3
+; LE-PWR8-NEXT:mr r9, r4
+; LE-PWR8-NEXT:mr r8, r5
+; LE-PWR8-NEXT:stqcx. r8, 0, r3
+; LE-PWR8-NEXT:bne cr0, .LBB0_1
+; LE-PWR8-NEXT:  # %bb.2: # %entry
+; LE-PWR8-NEXT:lwsync
+; LE-PWR8-NEXT:mr r3, r7
+; LE-PWR8-NEXT:mr r4, r6
+; LE-PWR8-NEXT:blr
+;
+; AIX64-PWR8-LABEL: swap:
+; AIX64-PWR8:   # %bb.0: # %entry
+; AIX64-PWR8-NEXT:mflr r0
+; AIX64-PWR8-NEXT:std r0, 16(r1)
+; AIX64-PWR8-NEXT:stdu r1, -112(r1)
+; AIX64-PWR8-NEXT:sync
+; AIX64-PWR8-NEXT:bl .__sync_lock_test_and_set_16[PR]
+; AIX64-PWR8-NEXT:nop
+; AIX64-PWR8-NEXT:lwsync
+; AIX64-PWR8-NEXT:addi r1, r1, 112
+; AIX64-PWR8-NEXT:ld r0, 16(r1)
+; AIX64-PWR8-NEXT:mtlr r0
+; AIX64-PWR8-NEXT:blr
+;
+; AIX32-PWR8-LABEL: swap:
+; AIX32-PWR8:   # %bb.0: # %entry
+; AIX32-PWR8-NEXT:mflr r0
+; AIX32-PWR8-NEXT:stw r0, 4(r1)
+; AIX32-PWR8-NEXT:stwu r1, -48(r1)
+; AIX32-PWR8-NEXT:.cfi_def_cfa_offset 48
+; AIX32-PWR8-NEXT:.cfi_offset lr, 4
+; AIX32-PWR8-NEXT:mr r4, r3
+; AIX32-PWR8-NEXT:stw r7, 40(r1)
+; AIX32-PWR8-NEXT:stw r6, 36(r1)
+; AIX32-PWR8-NEXT:addi r6, r1, 16
+; AIX32-PWR8-NEXT:li r3, 16
+; AIX32-PWR8-NEXT:li r7, 5
+; AIX32-PWR8-NEXT:stw r5, 32(r1)
+; AIX32-PWR8-NEXT:addi r5, r1, 32
+; AIX32-PWR8-NEXT:stw r8, 44(r1)
+; AIX32-PWR8-NEXT:bl __atomic_exchange
+; AIX32-PWR8-NEXT:lwz r6, 28(r1)
+; AIX32-PWR8-NEXT:lwz r5, 24(r1)
+; AIX32-PWR8-NEXT:lwz r4, 20(r1)
+; AIX32-PWR8-NEXT:lwz r3, 16(r1)
+; AIX32-PWR8-NEXT:lwz r0, 52(r1)
+; AIX32-PWR8-NEXT:addi r1, r1, 48
+; AIX32-PWR8-NEXT:mtlr r0
+; AIX32-PWR8-NEXT:blr
 entry:
   %0 = atomicrmw xchg i128* %a, i128 %x seq_cst, align 16
   ret i128 %0
@@ -76,6 +144,109 @@
 ; PWR7-NEXT:ld r0, 16(r1)
 ; PWR7-NEXT:mtlr r0
 ; PWR7-NEXT:blr
+;
+; LE-PWR8-LABEL: add:
+; LE-PWR8:   # %bb.0: # %entry
+; LE-PWR8-NEXT:sync
+; LE-PWR8-NEXT:  .LBB1_1: # %entry
+; LE-PWR8-NEXT:#
+; LE-PWR8-NEXT:lqarx r6, 0, r3
+; LE-PWR8-NEXT:addc r9, r4, r7
+; LE-PWR8-NEXT:adde r8, r5, r6
+; LE-PWR8-NEXT:stqcx. r8, 0, r3
+; LE-PWR8-NEXT:bne cr0, .LBB1_1
+; LE-PWR8-NEXT:  # %bb.2: # %entry
+; LE-PWR8-NEXT:lwsync
+; LE-PWR8-NEXT:mr r3, r7
+; LE-PWR8-NEXT:mr r4, r6
+; LE-PWR8-NEXT:blr
+;
+; AIX64-PWR8-LABEL: add:
+; AIX64-PWR8:   # %bb.0: # %entry
+; AIX64-PWR8-NEXT:mflr r0
+; AIX64-PWR8-NEXT:std r0, 16(r1)
+; AIX64-PWR8-NEXT:stdu r1, -112(r1)
+; AIX64-PWR8-NEXT:sync
+; AIX64-PWR8-NEXT:bl .__sync_fetch_and_add_16[PR]
+; AIX64-PWR8-NEXT:nop

[PATCH] D122698: [clangd] Add support to extract method for ExtractFunction Tweak

[Fabio R. Sluzala via Phabricator via cfe-commits]

FabioRS updated this revision to Diff 419616.
FabioRS marked 6 inline comments as done.
FabioRS set the repository for this revision to rG LLVM Github Monorepo.
FabioRS added a comment.

  namespace ns { int y = 1; void foo(); }
  void ns::foo() {[[int x; y++; ]]}



  namespace ns { int y = 1; void foo(); }
  void extracted() {
  int x; y++;
  }
  void ns::foo() {extracted(); }

This is not working, but I think it's not the scope of this review request, 
what do you think?

(See the tests line 478)

  class SomeClass {
static ns::A::C::RType extracted();
  static C::RType f();
  };

This is valid code but the syntactic looks strange with all the 
namespaces/classes, I tested various contexts but was not fixed, do you have 
some ideia for me to do?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122698

Files:
  clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp
  clang-tools-extra/clangd/unittests/tweaks/ExtractFunctionTests.cpp

Index: clang-tools-extra/clangd/unittests/tweaks/ExtractFunctionTests.cpp
===
--- clang-tools-extra/clangd/unittests/tweaks/ExtractFunctionTests.cpp
+++ clang-tools-extra/clangd/unittests/tweaks/ExtractFunctionTests.cpp
@@ -114,16 +114,48 @@
   // Don't extract when we need to make a function as a parameter.
   EXPECT_THAT(apply("void f() { [[int a; f();]] }"), StartsWith("fail"));
 
-  // We don't extract from methods for now since they may involve multi-file
-  // edits
-  std::string MethodFailInput = R"cpp(
+  std::string MethodInput = R"cpp(
 class T {
   void f() {
 [[int x;]]
   }
 };
   )cpp";
-  EXPECT_EQ(apply(MethodFailInput), "unavailable");
+  std::string MethodCheckOutput = R"cpp(
+class T {
+  void extracted() {
+int x;
+}
+void f() {
+extracted();
+  }
+};
+  )cpp";
+  EXPECT_EQ(apply(MethodInput), MethodCheckOutput);
+
+  std::string OutOfLineMethodInput = R"cpp(
+class T {
+  void f();
+};
+
+void T::f() {
+  [[int x;]]
+}
+  )cpp";
+  std::string OutOfLineMethodCheckOutput = R"cpp(
+class T {
+  void extracted();
+void f();
+};
+
+void T::extracted() {
+int x;
+}
+void T::f() {
+  extracted();
+}
+  )cpp";
+  EXPECT_EQ(apply(OutOfLineMethodInput), OutOfLineMethodCheckOutput);
 
   // We don't extract from templated functions for now as templates are hard
   // to deal with.
@@ -159,6 +191,305 @@
   EXPECT_EQ(apply(CompoundFailInput), "unavailable");
 }
 
+TEST_F(ExtractFunctionTest, DifferentHeaderSourceTest) {
+  Header = R"cpp(
+class SomeClass {
+  void f();
+};
+  )cpp";
+
+  std::string OutOfLineSource = R"cpp(
+void SomeClass::f() {
+  [[int x;]]
+}
+  )cpp";
+
+  std::string OutOfLineSourceOutputCheck = R"cpp(
+void SomeClass::extracted() {
+int x;
+}
+void SomeClass::f() {
+  extracted();
+}
+  )cpp";
+
+  std::string HeaderOutputCheck = R"cpp(
+class SomeClass {
+  void extracted();
+void f();
+};
+  )cpp";
+
+  llvm::StringMap EditedFiles;
+
+  EXPECT_EQ(apply(OutOfLineSource, ), OutOfLineSourceOutputCheck);
+  EXPECT_EQ(EditedFiles.begin()->second, HeaderOutputCheck);
+}
+
+TEST_F(ExtractFunctionTest, DifferentFilesNestedTest) {
+  Header = R"cpp(
+class T {
+class SomeClass {
+  void f();
+};
+};
+  )cpp";
+
+  std::string NestedOutOfLineSource = R"cpp(
+void T::SomeClass::f() {
+  [[int x;]]
+}
+  )cpp";
+
+  std::string NestedOutOfLineSourceOutputCheck = R"cpp(
+void T::SomeClass::extracted() {
+int x;
+}
+void T::SomeClass::f() {
+  extracted();
+}
+  )cpp";
+
+  std::string NestedHeaderOutputCheck = R"cpp(
+class T {
+class SomeClass {
+  void extracted();
+void f();
+};
+};
+  )cpp";
+
+  llvm::StringMap EditedFiles;
+
+  EXPECT_EQ(apply(NestedOutOfLineSource, ),
+NestedOutOfLineSourceOutputCheck);
+  EXPECT_EQ(EditedFiles.begin()->second, NestedHeaderOutputCheck);
+}
+
+TEST_F(ExtractFunctionTest, ConstexprDifferentHeaderSourceTest) {
+  Header = R"cpp(
+class SomeClass {
+  constexpr void f() const;
+};
+  )cpp";
+
+  std::string OutOfLineSource = R"cpp(
+constexpr void SomeClass::f() const {
+  [[int x;]]
+}
+  )cpp";
+
+  std::string OutOfLineSourceOutputCheck = R"cpp(
+constexpr void SomeClass::extracted() const {
+int x;
+}
+constexpr void SomeClass::f() const {
+  extracted();
+}
+  )cpp";
+
+  std::string HeaderOutputCheck = R"cpp(
+class SomeClass {
+  constexpr void extracted() const;
+constexpr void f() const;
+};
+  )cpp";
+
+  llvm::StringMap EditedFiles;
+
+  EXPECT_EQ(apply(OutOfLineSource, ), OutOfLineSourceOutputCheck);
+  EXPECT_NE(EditedFiles.begin(), EditedFiles.end())
+  << "The header should be edited and receives the declaration of the new "
+ "function";
+
+  

[PATCH] D122781: This patch aims to conform AMDGPUOpenMP driver sanitizer changes w.r.t HIPAMD toolchain.

[Amit Kumar Pandey via Phabricator via cfe-commits]

ampandey-AMD updated this revision to Diff 419617.
ampandey-AMD added a comment.

Updated the title


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122781

Files:
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/lib/Driver/ToolChains/AMDGPU.cpp
  clang/lib/Driver/ToolChains/AMDGPU.h
  clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp
  clang/lib/Driver/ToolChains/AMDGPUOpenMP.h
  clang/lib/Driver/ToolChains/HIPAMD.cpp
  clang/test/Driver/hip-sanitize-options.hip

Index: clang/test/Driver/hip-sanitize-options.hip
===
--- clang/test/Driver/hip-sanitize-options.hip
+++ clang/test/Driver/hip-sanitize-options.hip
@@ -62,7 +62,7 @@
 // RDC: {{"[^"]*clang[^"]*".* "-emit-llvm-bc".* "-fcuda-is-device".* "-mlink-bitcode-file" ".*asanrtl.bc".* "-mlink-builtin-bitcode" ".*hip.bc".* "-fsanitize=address".*}} "-o" "[[OUT:[^"]*.bc]]"
 // RDC-NOT: {{"[^"]*lld(\.exe){0,1}".*}} "[[OUT]]" {{".*asanrtl.bc" ".*hip.bc"}}
 
-// FAIL: AMDGPU address sanitizer runtime library (asanrtl) is not found. Please install ROCm device library which supports address sanitizer
+// FAIL: error: AMDGPU address sanitizer runtime library (asanrtl) is not found. Please install ROCm device library which supports address sanitizer
 
 // XNACK-DAG: warning: ignoring '-fsanitize=leak' option as it is not currently supported for target 'amdgcn-amd-amdhsa'
 // XNACK-DAG: warning: ignoring '-fsanitize=address' option as it is not currently supported for offload arch 'gfx900:xnack-'. Use it with an offload arch containing 'xnack+' instead
Index: clang/lib/Driver/ToolChains/HIPAMD.cpp
===
--- clang/lib/Driver/ToolChains/HIPAMD.cpp
+++ clang/lib/Driver/ToolChains/HIPAMD.cpp
@@ -35,43 +35,6 @@
 #define NULL_FILE "/dev/null"
 #endif
 
-static bool shouldSkipSanitizeOption(const ToolChain ,
- const llvm::opt::ArgList ,
- StringRef TargetID,
- const llvm::opt::Arg *A) {
-  // For actions without targetID, do nothing.
-  if (TargetID.empty())
-return false;
-  Option O = A->getOption();
-  if (!O.matches(options::OPT_fsanitize_EQ))
-return false;
-
-  if (!DriverArgs.hasFlag(options::OPT_fgpu_sanitize,
-  options::OPT_fno_gpu_sanitize, true))
-return true;
-
-  auto  = TC.getDriver().getDiags();
-
-  // For simplicity, we only allow -fsanitize=address
-  SanitizerMask K = parseSanitizerValue(A->getValue(), /*AllowGroups=*/false);
-  if (K != SanitizerKind::Address)
-return true;
-
-  llvm::StringMap FeatureMap;
-  auto OptionalGpuArch = parseTargetID(TC.getTriple(), TargetID, );
-
-  assert(OptionalGpuArch && "Invalid Target ID");
-  (void)OptionalGpuArch;
-  auto Loc = FeatureMap.find("xnack");
-  if (Loc == FeatureMap.end() || !Loc->second) {
-Diags.Report(
-clang::diag::warn_drv_unsupported_option_for_offload_arch_req_feature)
-<< A->getAsString(DriverArgs) << TargetID << "xnack+";
-return true;
-  }
-  return false;
-}
-
 void AMDGCN::Linker::constructLldCommand(Compilation , const JobAction ,
  const InputInfoList ,
  const InputInfo ,
@@ -337,12 +300,7 @@
 getSanitizerArgs(DriverArgs).needsAsanRt()) {
   auto AsanRTL = RocmInstallation.getAsanRTLPath();
   if (AsanRTL.empty()) {
-unsigned DiagID = getDriver().getDiags().getCustomDiagID(
-DiagnosticsEngine::Error,
-"AMDGPU address sanitizer runtime library (asanrtl) is not found. "
-"Please install ROCm device library which supports address "
-"sanitizer");
-getDriver().Diag(DiagID);
+getDriver().Diag(diag::err_drv_no_asan_rt_lib);
 return {};
   } else
 BCLibs.push_back({AsanRTL.str(), /*ShouldInternalize=*/false});
Index: clang/lib/Driver/ToolChains/AMDGPUOpenMP.h
===
--- clang/lib/Driver/ToolChains/AMDGPUOpenMP.h
+++ clang/lib/Driver/ToolChains/AMDGPUOpenMP.h
@@ -93,6 +93,10 @@
 
   SanitizerMask getSupportedSanitizers() const override;
 
+  RocmInstallationDetector getRocmInstallationLoc() const {
+return RocmInstallation;
+  }
+
   VersionTuple
   computeMSVCVersion(const Driver *D,
  const llvm::opt::ArgList ) const override;
Index: clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp
===
--- clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp
+++ clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp
@@ -16,6 +16,7 @@
 #include "clang/Driver/DriverDiagnostic.h"
 #include "clang/Driver/InputInfo.h"
 #include "clang/Driver/Options.h"
+#include "clang/Driver/SanitizerArgs.h"
 #include "clang/Driver/Tool.h"
 

[PATCH] D122608: Fix behavior of ifuncs with 'used' extern "C" static functions

[Tom Honermann via Phabricator via cfe-commits]

tahonermann added inline comments.



Comment at: clang/lib/CodeGen/CodeGenModule.cpp:6348-6349
+} else {
+  // If neither of these things, we have a user we don't know how to 
handle,
+  // so default to previous behavior of emitting a terrible error message.
+  return false;

This comment can only be understood in the context of this commit with the 
FIXME comment noted in the relevant test case. I suggest:
  // This user is one we don't know how to handle, so fail redirection. This
  // will result in an ifunc retaining a resolver name that will ultimately fail
  // to be resolved to a defined function.



Comment at: clang/lib/CodeGen/CodeGenModule.cpp:6390-6391
 llvm::GlobalValue *Val = I.second;
-if (Val && !getModule().getNamedValue(Name->getName()))
+llvm::GlobalValue *ExistingElem =
+getModule().getNamedValue(Name->getName());
+

I suggest moving the declaration of `ExistingElem` after the break on `!Val` 
given that the former is not relevant if `Val` is null.

I think the `!Val` check is worth a comment here. I suggest:
  // If Val is null, that implies that there were multiple declarations that 
each
  // had a claim to the unmangled name. In this case, generation of the alias
  // is suppressed. See CodeGenModule::MaybeHandleStaticInExternC().

The lack of checking for an existing element with the desired alias name was a 
pre-existing oversight, yes?



Comment at: clang/lib/CodeGen/CodeGenModule.cpp:6396-6397
+
+// Simple case, where there are no ifuncs involved.
+if (!ExistingElem || CheckAndReplaceExternCIFuncs(ExistingElem, Val))
   addCompilerUsedGlobal(llvm::GlobalAlias::create(Name->getName(), Val));

This comment seems inconsistent with the code. If there are no ifuncs involved, 
then why call `CheckAndReplaceExternCIFuncs()`?



Comment at: clang/lib/CodeGen/CodeGenModule.h:1573-1575
+  /// Helper function for EmitStaticExternCAliases that clears the uses of
+  /// 'Elem' if it is used exclusively by ifunc resolvers. Returns 'true' if it
+  /// was successful erases Elem.

Grammar is off in the last sentence.

The comment doesn't really explain this function's purpose. I suggest:
  /// Helper function for EmitStaticExternCAliases() to redirect ifuncs that 
have a resolver
  /// name that matches 'Elem' to instead resolve to the name of 'CppFunc'. This
  /// redirection is necessary in cases where 'Elem' has a name that will be 
emitted as
  /// an alias of the name bound to 'CppFunc'; ifuncs may not reference 
aliases. Redirection
  /// is only performed if 'Elem' is only used by ifuncs in which case, 'Elem' 
is destroyed..
  /// 'true' is returned If redirection is successful, and 'false' is returned 
otherwise.



Comment at: clang/test/SemaCXX/externc-ifunc-resolver.cpp:9-13
+// FIXME: This diagnostic is pretty confusing, the issue is that the existence
+// of the two functions suppresses the 'alias' creation, and thus the ifunc
+// resolution via the alias as well. In the future we should probably find
+// some way to improve this diagnostic (likely by diagnosing when we decide
+// this case suppresses alias creation).

Thanks for adding this comment; it is helpful.


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

https://reviews.llvm.org/D122608

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

[PATCH] D122822: [Clang][CodeGen]Add constant array support for __builtin_dump_sturct

[Wang Yihan via Phabricator via cfe-commits]

yihanaa added a comment.

I also agree that this is a useful debugging builtin, and no program logic 
should be forced to depend on the output of this builtin. It's just used to 
print out some useful debugging information


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122822

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

[PATCH] D122822: [Clang][CodeGen]Add constant array support for __builtin_dump_sturct

[Wang Yihan via Phabricator via cfe-commits]

yihanaa added inline comments.



Comment at: clang/lib/CodeGen/CGBuiltin.cpp:2048
+  QualType Type) {
+  static llvm::DenseMap Types;
   if (Types.empty()) {

erichkeane wrote:
> yihanaa wrote:
> > erichkeane wrote:
> > > yihanaa wrote:
> > > > erichkeane wrote:
> > > > > yihanaa wrote:
> > > > > > yihanaa wrote:
> > > > > > > erichkeane wrote:
> > > > > > > > yihanaa wrote:
> > > > > > > > > erichkeane wrote:
> > > > > > > > > > yihanaa wrote:
> > > > > > > > > > > erichkeane wrote:
> > > > > > > > > > > > Instead of this initialization, can we make this a 
> > > > > > > > > > > > constexpr constructor/collection of some sort and 
> > > > > > > > > > > > instantiate it inline?  
> > > > > > > > > > > > Instead of this initialization, can we make this a 
> > > > > > > > > > > > constexpr constructor/collection of some sort and 
> > > > > > > > > > > > instantiate it inline?  
> > > > > > > > > > > 
> > > > > > > > > > > I don't have a good idea because it relies on Context to 
> > > > > > > > > > > get some types like const char *
> > > > > > > > > > We could at minimum do a in inline-initializer instead of 
> > > > > > > > > > the branch below.
> > > > > > > > > I still haven't come up with a good idea to do it
> > > > > > > > Hrmph, `DenseMap` doesn't seem to have an init list ctor?!
> > > > > > > > 
> > > > > > > > Since this is a finite, known at compile time list of low 'N', 
> > > > > > > > I'd suggest just making it `static std::array > > > > > > > StringRef>> Types = {{Context.CharTy, "%c"},...`.
> > > > > > > > 
> > > > > > > > Then just doing a llvm::find_if.  The list is small enough I 
> > > > > > > > suspect the DenseMap overhead is more than the use of 
> > > > > > > > `llvm::find_if`.
> > > > > > > Haha,DenseMap also has no constexpr ctor. I think the time 
> > > > > > > complexity of these two is the same. 
> > > > > > The Context.CharTy... etc. are not static, So we might need a 
> > > > > > static value instead of it, and compare it with QualType
> > > > > They don't have to be static?  This should do 'magic static' 
> > > > > initialization.
> > > > I can't agree more. it might be a type identifier, such as 
> > > > getAsString()'s return value
> > > Actually, I don't think the collection can be static!  When we re-use the 
> > > same process for multiple TUs, we get a new ASTContext.  SO this would 
> > > result in us having types that don't necessarily 'match'.  I think this 
> > > collection/checks need to either be stored in ASTContext, or in an 
> > > 'if-else' tree here.
> > I also don't think it should be maintained in this place. Shouldn't this be 
> > better maintained together with the printf parameter checking part, what do 
> > you think?
> I'm unfamiliar with what you mean.
> 
> HOWEVER, the comments by @rsmith and @rjmccall I think are blocking on this 
> feature/functionality anyway.  So we might still wish to refactor this 
> builtin significantly based on what they come up with.
Ah, I think the way I described it is not correct, my thought is, can this 
getDumpStructFormatSpecifier similar function be maintained in somewhere like 
FormatString.h


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122822

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

[PATCH] D121271: [C++20] [Modules] Don't generate strong function of a partition in importing modules

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu added a comment.

@iains now it passes the CI!


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

https://reviews.llvm.org/D121271

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

[PATCH] D119544: Deferred Concept Instantiation Implementation

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu added a comment.

BTW, it looks like the patch needs to rebase with main so that other people 
could play it self if interested.




Comment at: clang/lib/Sema/SemaConcept.cpp:489
+  // Attn Reviewers: we need to do this for the function constraints for
+  // comparison of constraints to work, but do we also need to do it for
+  // CheckInstantiatedFunctionConstraints?  That one is more difficult, but we

erichkeane wrote:
> ChuanqiXu wrote:
> > Would you mind to elaborate for the issue "function constraints for 
> > comparison of constraints to work" more? Maybe it is said in previous 
> > messages, but the history is hard to follow...
> Yep, this one is difficult :/  Basically, when we instantiate the constraints 
> at 'checking' time, if the function is NOT a template, we call 
> `CheckFunctionConstraints`. When we go to check the 'subsumption' type things 
> with a fully instantiated version, they fail if they are dependent (as it is 
> expected that way).  See the `setTrailingRequiresClause` here.
> 
> HOWEVER, it seems we ALWAYS pick constraints off the primary template, rather 
> than the 'stored' version of the constraint on the current declaration.  I 
> tried to do something similar here for those cases, but 1: it had no effect 
> seemingly, and 2: it ended up getting complicated in many cases (as figuring 
> out the relationship between the constraints in the two nodes was 
> difficult/near impossible).
> 
> I opted to not do it, and I don't THINK it needs to happen over there, but I 
> wanted to point out that I was skipping it in case reviewers had a better 
> idea.
> 
> 
Let me ask some questions to get in consensus for the problem:

> Basically, when we instantiate the constraints at 'checking' time, if the 
> function is NOT a template, we call CheckFunctionConstraints.

I think the function who contains a trailing require clause should be template 
one. Do you want to say independent ? Or do you refer the following case?

```C++
template struct A {
  static void f(int) requires ;
};
```

And the related question would be what's the cases that 
`CheckFunctionConstraints` would be called and what's the cases that 
`CheckinstantiatedFunctionTemplateConstraints` would be called?

> When we go to check the 'subsumption' type things with a fully instantiated 
> version, they fail if they are dependent (as it is expected that way).

If it is fully instantiated, how could it be dependent?

> HOWEVER, it seems we ALWAYS pick constraints off the primary template, rather 
> than the 'stored' version of the constraint on the current declaration.

1. What is `we`? I mean what's the function would pick up the constraints from 
the primary template.
2. Does `the stored version of the constraint` means the trailing require 
clause of FD? Would it be the original one or `Converted[0]`.


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

https://reviews.llvm.org/D119544

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

[clang] f1b0a4f - An expression should only contain an unexpanded parameter pack if it

[Richard Smith via cfe-commits]

Author: Richard Smith
Date: 2022-03-31T20:02:53-07:00
New Revision: f1b0a4fc540f986372f09768c325d505b75b414d

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

LOG: An expression should only contain an unexpanded parameter pack if it
lexically contains a mention of the pack.

Systematically distinguish between syntactic and semantic references to
packs, especially when propagating dependence from a type into an
expression. We should consult the type-as-written when computing
syntactic dependence and should consult the semantic type when computing
semantic dependence.

Fixes #54402.

Added: 
clang/test/SemaTemplate/find-unexpanded-packs.cpp

Modified: 
clang/include/clang/AST/ComputeDependence.h
clang/include/clang/AST/DependenceFlags.h
clang/include/clang/AST/Expr.h
clang/lib/AST/ComputeDependence.cpp
clang/lib/AST/ExprCXX.cpp
clang/lib/AST/Type.cpp
clang/test/SemaCXX/cxx1z-class-template-argument-deduction.cpp

Removed: 




diff  --git a/clang/include/clang/AST/ComputeDependence.h 
b/clang/include/clang/AST/ComputeDependence.h
index cb545aff51f86..3360fd11ab1f7 100644
--- a/clang/include/clang/AST/ComputeDependence.h
+++ b/clang/include/clang/AST/ComputeDependence.h
@@ -30,7 +30,8 @@ class UnaryExprOrTypeTraitExpr;
 class ArraySubscriptExpr;
 class MatrixSubscriptExpr;
 class CompoundLiteralExpr;
-class CastExpr;
+class ImplicitCastExpr;
+class ExplicitCastExpr;
 class BinaryOperator;
 class ConditionalOperator;
 class BinaryConditionalOperator;
@@ -70,6 +71,7 @@ class CXXPseudoDestructorExpr;
 class OverloadExpr;
 class DependentScopeDeclRefExpr;
 class CXXConstructExpr;
+class CXXTemporaryObjectExpr;
 class CXXDefaultInitExpr;
 class CXXDefaultArgExpr;
 class LambdaExpr;
@@ -114,7 +116,8 @@ ExprDependence computeDependence(UnaryExprOrTypeTraitExpr 
*E);
 ExprDependence computeDependence(ArraySubscriptExpr *E);
 ExprDependence computeDependence(MatrixSubscriptExpr *E);
 ExprDependence computeDependence(CompoundLiteralExpr *E);
-ExprDependence computeDependence(CastExpr *E);
+ExprDependence computeDependence(ImplicitCastExpr *E);
+ExprDependence computeDependence(ExplicitCastExpr *E);
 ExprDependence computeDependence(BinaryOperator *E);
 ExprDependence computeDependence(ConditionalOperator *E);
 ExprDependence computeDependence(BinaryConditionalOperator *E);
@@ -156,6 +159,7 @@ ExprDependence computeDependence(OverloadExpr *E, bool 
KnownDependent,
  bool KnownContainsUnexpandedParameterPack);
 ExprDependence computeDependence(DependentScopeDeclRefExpr *E);
 ExprDependence computeDependence(CXXConstructExpr *E);
+ExprDependence computeDependence(CXXTemporaryObjectExpr *E);
 ExprDependence computeDependence(CXXDefaultInitExpr *E);
 ExprDependence computeDependence(CXXDefaultArgExpr *E);
 ExprDependence computeDependence(LambdaExpr *E,

diff  --git a/clang/include/clang/AST/DependenceFlags.h 
b/clang/include/clang/AST/DependenceFlags.h
index 62efdb4ce6e44..3b3c1afb096ad 100644
--- a/clang/include/clang/AST/DependenceFlags.h
+++ b/clang/include/clang/AST/DependenceFlags.h
@@ -130,6 +130,14 @@ class Dependence {
 
 // Dependence that is propagated syntactically, regardless of semantics.
 Syntactic = UnexpandedPack | Instantiation | Error,
+// Dependence that is propagated semantically, even in cases where the
+// type doesn't syntactically appear. This currently excludes only
+// UnexpandedPack. Even though Instantiation dependence is also notionally
+// syntactic, we also want to propagate it semantically because anything
+// that semantically depends on an instantiation-dependent entity should
+// always be instantiated when that instantiation-dependent entity is.
+Semantic =
+Instantiation | Type | Value | Dependent | Error | VariablyModified,
 
 LLVM_MARK_AS_BITMASK_ENUM(/*LargestValue=*/VariablyModified)
   };
@@ -175,6 +183,14 @@ class Dependence {
 return Result;
   }
 
+  /// Extract the semantic portions of this type's dependence that apply even
+  /// to uses where the type does not appear syntactically.
+  Dependence semantic() {
+Dependence Result = *this;
+Result.V &= Semantic;
+return Result;
+  }
+
   TypeDependence type() const {
 return translate(V, UnexpandedPack, TypeDependence::UnexpandedPack) |
translate(V, Instantiation, TypeDependence::Instantiation) |
@@ -231,7 +247,10 @@ class Dependence {
 inline ExprDependence toExprDependence(TemplateArgumentDependence TA) {
   return Dependence(TA).expr();
 }
-inline ExprDependence toExprDependence(TypeDependence D) {
+inline ExprDependence toExprDependenceForImpliedType(TypeDependence D) {
+  return Dependence(D).semantic().expr();
+}
+inline ExprDependence 

[PATCH] D121271: [C++20] [Modules] Don't generate strong function of a partition in importing modules

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu updated this revision to Diff 419603.
ChuanqiXu added a comment.

Specify itanium_abi_triple to make CI on windows passes.


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

https://reviews.llvm.org/D121271

Files:
  clang/include/clang/Basic/Module.h
  clang/lib/Serialization/ASTWriterDecl.cpp
  clang/test/CodeGenCXX/partitions.cpp

Index: clang/test/CodeGenCXX/partitions.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/partitions.cpp
@@ -0,0 +1,50 @@
+// RUN: rm -rf %t
+// RUN: mkdir %t
+// RUN: split-file %s %t
+
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface -triple %itanium_abi_triple %t/parta.cppm -o %t/mod-parta.pcm
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface -triple %itanium_abi_triple %t/partb.cppm -o %t/mod-partb.pcm
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface -triple %itanium_abi_triple -fmodule-file=%t/mod-parta.pcm \
+// RUN: -fmodule-file=%t/mod-partb.pcm %t/mod.cppm -o %t/mod.pcm
+// RUN: %clang_cc1 -std=c++20 -triple %itanium_abi_triple %t/mod.pcm -S -emit-llvm -disable-llvm-passes -o - \
+// RUN: | FileCheck %t/mod.cppm
+// RUN: %clang_cc1 -std=c++20 -O2 -emit-module-interface -triple %itanium_abi_triple -fmodule-file=%t/mod-parta.pcm \
+// RUN: -fmodule-file=%t/mod-partb.pcm %t/mod.cppm -o %t/mod.pcm
+// RUN: %clang_cc1 -std=c++20 -O2 -triple %itanium_abi_triple %t/mod.pcm -S -emit-llvm -disable-llvm-passes -o - \
+// RUN: | FileCheck %t/mod.cppm  -check-prefix=CHECK-OPT
+
+//--- parta.cppm
+export module mod:parta;
+
+export int a = 43;
+
+export int foo() {
+  return 3 + a;
+}
+
+//--- partb.cppm
+module mod:partb;
+
+int b = 43;
+
+int bar() {
+  return 43 + b;
+}
+
+//--- mod.cppm
+export module mod;
+import :parta;
+import :partb;
+export int use() {
+  return foo() + bar() + a + b;
+}
+
+// CHECK: @_ZW3mod1a = available_externally global
+// CHECK: @_ZW3mod1b = available_externally global
+// CHECK: declare{{.*}} i32 @_ZW3mod3foov
+// CHECK: declare{{.*}} i32 @_ZW3mod3barv
+
+// CHECK-OPT: @_ZW3mod1a = available_externally global
+// CHECK-OPT: @_ZW3mod1b = available_externally global
+// CHECK-OPT: define available_externally{{.*}} i32 @_ZW3mod3foov
+// CHECK-OPT: define available_externally{{.*}} i32 @_ZW3mod3barv
Index: clang/lib/Serialization/ASTWriterDecl.cpp
===
--- clang/lib/Serialization/ASTWriterDecl.cpp
+++ clang/lib/Serialization/ASTWriterDecl.cpp
@@ -1029,12 +1029,12 @@
 if (Writer.WritingModule &&
 !D->getDescribedVarTemplate() && !D->getMemberSpecializationInfo() &&
 !isa(D)) {
-  // When building a C++ Modules TS module interface unit, a strong
-  // definition in the module interface is provided by the compilation of
-  // that module interface unit, not by its users. (Inline variables are
-  // still emitted in module users.)
+  // When building a C++20 module interface unit or a partition unit, a
+  // strong definition in the module interface is provided by the
+  // compilation of that unit, not by its users. (Inline variables are still
+  // emitted in module users.)
   ModulesCodegen =
-  (Writer.WritingModule->Kind == Module::ModuleInterfaceUnit ||
+  (Writer.WritingModule->isInterfaceOrPartition() ||
(D->hasAttr() &&
 Writer.Context->getLangOpts().BuildingPCHWithObjectFile)) &&
   Writer.Context->GetGVALinkageForVariable(D) == GVA_StrongExternal;
@@ -2470,11 +2470,11 @@
   if (!FD->isDependentContext()) {
 Optional Linkage;
 if (Writer->WritingModule &&
-Writer->WritingModule->Kind == Module::ModuleInterfaceUnit) {
-  // When building a C++ Modules TS module interface unit, a strong
-  // definition in the module interface is provided by the compilation of
-  // that module interface unit, not by its users. (Inline functions are
-  // still emitted in module users.)
+Writer->WritingModule->isInterfaceOrPartition()) {
+  // When building a C++20 module interface unit or a partition unit, a
+  // strong definition in the module interface is provided by the
+  // compilation of that unit, not by its users. (Inline functions are still
+  // emitted in module users.)
   Linkage = Writer->Context->GetGVALinkageForFunction(FD);
   ModulesCodegen = *Linkage == GVA_StrongExternal;
 }
Index: clang/include/clang/Basic/Module.h
===
--- clang/include/clang/Basic/Module.h
+++ clang/include/clang/Basic/Module.h
@@ -529,6 +529,10 @@
 
   /// Is this module a header unit.
   bool isHeaderUnit() const { return Kind == ModuleHeaderUnit; }
+  // Is this a C++20 module interface or a partition.
+  bool isInterfaceOrPartition() const {
+return Kind == ModuleInterfaceUnit || isModulePartition();
+  }
 
   /// Get the primary module interface name from 

[PATCH] D122377: [PowerPC][Linux] Support 16-byte lock free atomics on pwr8 and up

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added inline comments.



Comment at: clang/lib/Basic/Targets/PPC.h:448
+  void setMaxAtomicWidth() override {
+// For layout on ELF targets, we support up to 16 bytes.
+if (getTriple().isOSBinFormatELF())

I believe this should be presented more as this override being implemented 
currently only for ELF targets. The current presentation seems to imply more 
design intent for non-ELF targets than there is consensus for.

For example:
```
if (!getTriple().isOSBinFormatELF())
  return PPCTargetInfo::setMaxAtomicWidth();
```



Comment at: clang/test/Sema/atomic-ops.c:13
+// RUN:   -target-cpu pwr8 -DPPC64_PWR8
 
 // Basic parsing/Sema tests for __c11_atomic_*

This will need a separate patch to cover ppc32 (likely with AIX).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122377

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

[PATCH] D122766: [clang] Use forward slash as the path separator for Windows in __FILE__, __builtin_FILE(), and std::source_location

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

Do you know what cl.exe does to paths in pdb files? Does it write 
mixed-slashiness for foo/bar.h includes, or does it write backslashes 
throughout?

Windows can handle slashes, but several tools can't. I worry that if we do 
something different than cl, some random corner case might break (dbghelp, or 
some source server thing or some custom debug info processor somewhere).

> Then secondly - if the source paths are relative paths, making them 
> OS-agnostic in this way does make sense. But if they are absolute, they are 
> pretty much by definition specific to the build host,

We use `/pdbsourcepath:X:\fake\prefix` to write a deterministic absolute path 
to the output file at link time (and `-fdebug-compilation-dir .` to get 
deterministic compiler output – see 
https://blog.llvm.org/2019/11/deterministic-builds-with-clang-and-lld.html). 
The motivation is to get exactly the same output when building on linux and 
windows hosts.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122766

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

[PATCH] D119544: Deferred Concept Instantiation Implementation

[Erich Keane via Phabricator via cfe-commits]

erichkeane added inline comments.



Comment at: clang/include/clang/Sema/Sema.h:7011
+
+  bool CheckConstraintSatisfaction(
+  const NamedDecl *Template, ArrayRef ConstraintExprs,

ChuanqiXu wrote:
> I think this one need comment too. What's the difference with the above one?
Thanks! I'll comment on that.  The difference is the 'out' parameter of 
'ConvertedConstraints', but I'll put details in the comment.



Comment at: clang/lib/Sema/SemaConcept.cpp:489
+  // Attn Reviewers: we need to do this for the function constraints for
+  // comparison of constraints to work, but do we also need to do it for
+  // CheckInstantiatedFunctionConstraints?  That one is more difficult, but we

ChuanqiXu wrote:
> Would you mind to elaborate for the issue "function constraints for 
> comparison of constraints to work" more? Maybe it is said in previous 
> messages, but the history is hard to follow...
Yep, this one is difficult :/  Basically, when we instantiate the constraints 
at 'checking' time, if the function is NOT a template, we call 
`CheckFunctionConstraints`. When we go to check the 'subsumption' type things 
with a fully instantiated version, they fail if they are dependent (as it is 
expected that way).  See the `setTrailingRequiresClause` here.

HOWEVER, it seems we ALWAYS pick constraints off the primary template, rather 
than the 'stored' version of the constraint on the current declaration.  I 
tried to do something similar here for those cases, but 1: it had no effect 
seemingly, and 2: it ended up getting complicated in many cases (as figuring 
out the relationship between the constraints in the two nodes was 
difficult/near impossible).

I opted to not do it, and I don't THINK it needs to happen over there, but I 
wanted to point out that I was skipping it in case reviewers had a better idea.




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

https://reviews.llvm.org/D119544

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

[PATCH] D117835: [OpenMPIRBuilder] Detect and fix ambiguous InsertPoints for createSections.

[Peixin Qiao via Phabricator via cfe-commits]

peixin added a comment.

Added @shraiysh and @NimishMishra . They may be more familiar to this code.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117835

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

[PATCH] D122104: [X86][regcall] Support passing / returning structures

[Phoebe Wang via Phabricator via cfe-commits]

pengfei added a comment.

Thanks @erichkeane @aaron.ballman ! Yeah, I didn't receive buildbots notice 
about that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122104

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

[PATCH] D120305: [Driver] Default CLANG_DEFAULT_PIE_ON_LINUX to ON

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.
Herald added a subscriber: StephenFan.

@nemanjai mentioned "All the issues I was seeing seem to have been resolved. I 
can build with CLANG_DEFAULT_PIE_ON_LINUX=on without any failing sanitizer or 
crt failures."
Will recommit. And ppc64 bots have hopefully got better with the upgrade. 
Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120305

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

[PATCH] D122841: [analyzer] Consider all addrspaces in null dereference check

[Vince Bridgers via Phabricator via cfe-commits]

vabridgers added a comment.

@steakhal, good question. Digging into the change archaeology, I see this 
change was made by Anna Zaks in Jan 2016 - see commit header below.

There's no reference to a review with comments, and I don't know how to find 
those if they exist, perhaps @NoQ could help with that?

In any case, judging by the changes I had to make to the test cases, there were 
no regressions in the current test set, and at least our build tests ran ok - 
seems like this is no longer necessary. But I understand the need to approach 
this carefully.

Thanks

  commit c9f16fe48c40e7683d0029b65090d0007414d7af
  Author: Anna Zaks 
  Date:   Wed Jan 6 00:32:49 2016 +
  
  [analyzer] Don't report null dereferences on address_space annotated 
memory
  
  llvm-svn: 256885


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122841

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

[PATCH] D115907: [misexpect] Re-implement MisExpect Diagnostics

[Paul Kirth via Phabricator via cfe-commits]

paulkirth added a comment.

@jgorbe Sorry for the trouble. Thank you for the backtrace and the revert. 
Indeed, it seems like I've missed an assumption w.r.t.  over/underflow, and 
will have to sort that out before re-landing.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115907

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

[PATCH] D122377: [PowerPC][Linux] Support 16-byte lock free atomics on pwr8 and up

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added inline comments.



Comment at: clang/test/CodeGen/PowerPC/atomic-alignment.c:34
 
 // PPC32: @o = global %struct.O zeroinitializer, align 1{{$}}
 // PPC64: @o = global %struct.O zeroinitializer, align 8{{$}}

Just noting that GCC increases the alignment even for ppc32:
```
typedef struct A8 { char x[8]; } A8;
typedef struct A16 { char x[16]; } A16;
extern char q8[_Alignof(_Atomic(A8))], q8[8]; // okay for GCC targeting ppc32
extern char q16[_Alignof(_Atomic(A16))], q16[16]; // okay for GCC targeting 
ppc32
```

Apparently, the change for i686 in GCC occurred with version 11.
https://godbolt.org/z/fTTGoqWW1


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122377

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

[PATCH] D122248: [clang][CodeGen]Fix clang crash and add bitfield support in __builtin_dump_struct

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

rsmith added inline comments.



Comment at: clang/lib/CodeGen/CGBuiltin.cpp:2113-2117
 if (CanonicalType->isRecordType()) {
-  TmpRes = dumpRecord(CGF, CanonicalType, FieldPtr, Align, Func, Lvl + 1);
+  TmpRes = dumpRecord(CGF, CanonicalType, FieldLV, Align, Func, Lvl + 1);
   Res = CGF.Builder.CreateAdd(TmpRes, Res);
   continue;
 }

After this patch, this case no longer prints the field name. Eg: 
https://godbolt.org/z/o7vcbWaEf

```
#include 

struct A {};

struct B {
A a;
};


int main() {
B x;
__builtin_dump_struct(, );
}
```

now prints:

```
B {
A {
}
}
```

This seems like an important regression; please can you take a look? This also 
suggests to me that there's a hole in our test coverage.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122248

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

[PATCH] D118409: [OpenMPIRBuilder] Remove ContinuationBB argument from Body callback.

[Michael Kruse via Phabricator via cfe-commits]

Meinersbur added a comment.
Herald added a project: All.

ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118409

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

[PATCH] D117835: [OpenMPIRBuilder] Detect and fix ambiguous InsertPoints for createSections.

[Michael Kruse via Phabricator via cfe-commits]

Meinersbur added a comment.
Herald added a project: All.

ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117835

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

[PATCH] D122852: [OPENMP] Fix assertion in clang::ASTContext::getTypeInfoImpl

[Jennifer Yu via Phabricator via cfe-commits]

jyu2 created this revision.
jyu2 added reviewers: ABataev, jdoerfert, mikerice.
jyu2 added a project: OpenMP.
Herald added subscribers: guansong, yaxunl.
Herald added a project: All.
jyu2 requested review of this revision.
Herald added a subscriber: sstefan1.

The problem is when mapping of array section, currently call
getTypeSizeInChars with BuiltinType::OMPArraySection causing assert.

One way to fix this is using array element type instead.

BTW with this fix, test will fail in libomptarget.so
with error message: double free or corruption (out)
But it passes with intel customized libomptarget.so

I am not sure it is clang problem.  I will submit issues to 
libtarget after this checked in for more investigation.


Repository:
  rC Clang

https://reviews.llvm.org/D122852

Files:
  clang/lib/CodeGen/CGOpenMPRuntime.cpp
  clang/test/OpenMP/target_map_codegen_36.cpp

Index: clang/test/OpenMP/target_map_codegen_36.cpp
===
--- /dev/null
+++ clang/test/OpenMP/target_map_codegen_36.cpp
@@ -0,0 +1,514 @@
+// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py
+// expected-no-diagnostics
+#ifndef HEADER
+#define HEADER
+
+///==///
+// RUN: %clang_cc1 -DCK36 -verify -fopenmp -fopenmp-version=50 -fopenmp-targets=powerpc64le-ibm-linux-gnu -x c++ -triple powerpc64le-unknown-unknown -emit-llvm %s -o - | FileCheck %s --check-prefix CK36 --check-prefix CK36-64
+// RUN: %clang_cc1 -DCK36 -fopenmp -fopenmp-version=50 -fopenmp-targets=powerpc64le-ibm-linux-gnu -x c++ -std=c++11 -triple powerpc64le-unknown-unknown -emit-pch -o %t %s
+// RUN: %clang_cc1 -fopenmp -fopenmp-version=50 -fopenmp-targets=powerpc64le-ibm-linux-gnu -x c++ -triple powerpc64le-unknown-unknown -std=c++11 -include-pch %t -verify %s -emit-llvm -o - | FileCheck %s  --check-prefix CK36 --check-prefix CK36-64
+// RUN: %clang_cc1 -DCK36 -verify -fopenmp -fopenmp-version=50 -fopenmp-targets=i386-pc-linux-gnu -x c++ -triple i386-unknown-unknown -emit-llvm %s -o - | FileCheck %s  --check-prefix CK36 --check-prefix CK36-32
+// RUN: %clang_cc1 -DCK36 -fopenmp -fopenmp-version=50 -fopenmp-targets=i386-pc-linux-gnu -x c++ -std=c++11 -triple i386-unknown-unknown -emit-pch -o %t %s
+// RUN: %clang_cc1 -fopenmp -fopenmp-version=50 -fopenmp-targets=i386-pc-linux-gnu -x c++ -triple i386-unknown-unknown -std=c++11 -include-pch %t -verify %s -emit-llvm -o - | FileCheck %s  --check-prefix CK36 --check-prefix CK36-32
+
+// RUN: %clang_cc1 -DCK36 -verify -fopenmp-simd -fopenmp-version=50 -fopenmp-targets=powerpc64le-ibm-linux-gnu -x c++ -triple powerpc64le-unknown-unknown -emit-llvm %s -o - | FileCheck --check-prefix SIMD-ONLY32 %s
+// RUN: %clang_cc1 -DCK36 -fopenmp-simd -fopenmp-version=50 -fopenmp-targets=powerpc64le-ibm-linux-gnu -x c++ -std=c++11 -triple powerpc64le-unknown-unknown -emit-pch -o %t %s
+// RUN: %clang_cc1 -fopenmp-simd -fopenmp-version=50 -fopenmp-targets=powerpc64le-ibm-linux-gnu -x c++ -triple powerpc64le-unknown-unknown -std=c++11 -include-pch %t -verify %s -emit-llvm -o - | FileCheck --check-prefix SIMD-ONLY32 %s
+// RUN: %clang_cc1 -DCK36 -verify -fopenmp-simd -fopenmp-version=50 -fopenmp-targets=i386-pc-linux-gnu -x c++ -triple i386-unknown-unknown -emit-llvm %s -o - | FileCheck --check-prefix SIMD-ONLY32 %s
+// RUN: %clang_cc1 -DCK36 -fopenmp-simd -fopenmp-version=50 -fopenmp-targets=i386-pc-linux-gnu -x c++ -std=c++11 -triple i386-unknown-unknown -emit-pch -o %t %s
+// RUN: %clang_cc1 -fopenmp-simd -fopenmp-version=50 -fopenmp-targets=i386-pc-linux-gnu -x c++ -triple i386-unknown-unknown -std=c++11 -include-pch %t -verify %s -emit-llvm -o - | FileCheck --check-prefix SIMD-ONLY32 %s
+// SIMD-ONLY32-NOT: {{__kmpc|__tgt}}
+#ifdef CK36
+typedef struct {
+  int a;
+  double *b;
+} C;
+#pragma omp declare mapper(C s) map(to : s.a) map(tofrom : s.b [0:2])
+
+typedef struct {
+  int e;
+  int h;
+  C f;
+} D;
+// CK36-DAG: [[SIZE_TO:@.+]] = private {{.*}}constant [4 x i64] [i64 0, i64 0, i64 0, i64 {{48|32}}]
+// CK36--DAG: [[MTYPE_TO:@.+]] = {{.+}}constant [4 x i64] [i64 32, i64 281474976710659, i64 281474976710659, i64 281474976711171]
+
+// CK36-64-LABEL: @_Z4testv(
+// CK36-64-NEXT:  entry:
+// CK36-64-NEXT:[[SA:%.*]] = alloca [10 x %struct.D], align 8
+// CK36-64-NEXT:[[X:%.*]] = alloca [2 x double], align 8
+// CK36-64-NEXT:[[Y:%.*]] = alloca [2 x double], align 8
+// CK36-64-NEXT:[[DOTOFFLOAD_BASEPTRS:%.*]] = alloca [4 x i8*], align 8
+// CK36-64-NEXT:[[DOTOFFLOAD_PTRS:%.*]] = alloca [4 x i8*], align 8
+// CK36-64-NEXT:[[DOTOFFLOAD_MAPPERS:%.*]] = alloca [4 x i8*], align 8
+// CK36-64-NEXT:[[DOTOFFLOAD_SIZES:%.*]] = alloca [4 x i64], align 8
+// CK36-64-NEXT:[[SAAA:%.*]] = alloca [10 x %struct.D], align 8
+// CK36-64-NEXT:[[SAA:%.*]] = alloca %struct.D*, align 8
+// CK36-64-NEXT:[[DOTOFFLOAD_BASEPTRS32:%.*]] = alloca [4 x i8*], align 8
+// 

[PATCH] D115907: [misexpect] Re-implement MisExpect Diagnostics

[Jorge Gorbe Moya via Phabricator via cfe-commits]

jgorbe added a comment.

By the way, the line number for `llvm::misexpect::verifyMisExpect` in the stack 
trace above includes the debug `printf`s I inserted to get the variable values 
so it won't match the exact line number in the repo. But I think that's the 
only call to `BranchProbability` in that function so I hope it's not very 
confusing anyway. Sorry about that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115907

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

[PATCH] D115907: [misexpect] Re-implement MisExpect Diagnostics

[Jorge Gorbe Moya via Phabricator via cfe-commits]

jgorbe added a comment.

Hi, this patch is causing floating point exceptions for us during profile 
generation. On a debug build I get this assertion failure (see stack trace 
below):

  clang: 
/usr/local/google/home/jgorbe/code/llvm/llvm/lib/Support/BranchProbability.cpp:41:
 llvm::BranchProbability::BranchProbability(uint32_t, uint32_t): Assertion 
`Denominator > 0 && "Denominator cannot be 0!"' failed.

Printing some values around the problem I got

  TotalBranchWeight = 4294967296   
  LikelyBranchWeight = 2147483648   

   
  UnlikelyBranchWeight = 2147483648 

   
  NumUnlikelyTargets = 1

I see the `BranchProbability` constructor takes `uint32_t`s, so this looks like 
it's overflowing to 0 (4294967296 is exactly 2**32).

I'm going to revert the patch to unbreak our build. Please let me know if you 
need more details and I'll try to come up with a reproducer I can share. Here's 
the stack trace for the assertion.

   #0 0x0a7f992a llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) 
/usr/local/google/home/jgorbe/code/llvm/llvm/lib/Support/Unix/Signals.inc:565:11
   #1 0x0a7f9afb PrintStackTraceSignalHandler(void*) 
/usr/local/google/home/jgorbe/code/llvm/llvm/lib/Support/Unix/Signals.inc:632:1 
  
   #2 0x0a7f80bb llvm::sys::RunSignalHandlers() 
/usr/local/google/home/jgorbe/code/llvm/llvm/lib/Support/Signals.cpp:102:5  
   
   #3 0x0a7fa271 SignalHandler(int) 
/usr/local/google/home/jgorbe/code/llvm/llvm/lib/Support/Unix/Signals.inc:407:1 

   
   #4 0x7ff57cfe2200 __restore_rt 
(/lib/x86_64-linux-gnu/libpthread.so.0+0x13200) 

 
   #5 0x7ff57ca678a1 raise ./signal/../sysdeps/unix/sysv/linux/raise.c:50:1 

   
   #6 0x7ff57ca51546 abort ./stdlib/abort.c:81:7

   
   #7 0x7ff57ca5142f get_sysdep_segment_value ./intl/loadmsgcat.c:509:8 

   
   #8 0x7ff57ca5142f _nl_load_domain ./intl/loadmsgcat.c:970:34 

   
   #9 0x7ff57ca60222 (/lib/x86_64-linux-gnu/libc.so.6+0x35222)  

   
  #10 0x0a6cb517 llvm::BranchProbability::BranchProbability(unsigned 
int, unsigned int) 
/usr/local/google/home/jgorbe/code/llvm/llvm/lib/Support/BranchProbability.cpp:0:3
 
  #11 0x0a937a4d llvm::misexpect::verifyMisExpect(llvm::Instruction&, 
llvm::ArrayRef, llvm::ArrayRef) 
/usr/local/google/home/jgorbe/code/llvm/llvm/lib/Transforms/Utils/MisExpect.cpp:190:54



  #12 0x0a937ef3 
llvm::misexpect::checkBackendInstrumentation(llvm::Instruction&, 
llvm::ArrayRef) 
/usr/local/google/home/jgorbe/code/llvm/llvm/lib/Transforms/Utils/MisExpect.cpp:217:1
  
  #13 0x0a93807b 
llvm::misexpect::checkExpectAnnotations(llvm::Instruction&, 
llvm::ArrayRef, bool) 
/usr/local/google/home/jgorbe/code/llvm/llvm/lib/Transforms/Utils/MisExpect.cpp:236:1
 
  #14 0x09e1339c (anonymous 
namespace)::SampleProfileLoader::generateMDProfMetadata(llvm::Function&) 
/usr/local/google/home/jgorbe/code/llvm/llvm/lib/Transforms/IPO/SampleProfile.cpp:1755:19
 
  #15 0x09e10d94 (anonymous 
namespace)::SampleProfileLoader::emitAnnotations(llvm::Function&) 
/usr/local/google/home/jgorbe/code/llvm/llvm/lib/Transforms/IPO/SampleProfile.cpp:0:5

  #16 0x09e1022b (anonymous 
namespace)::SampleProfileLoader::runOnFunction(llvm::Function&, 
llvm::AnalysisManager*) 
/usr/local/google/home/jgorbe/code/llvm/llvm/lib/Transforms/IPO/SampleProfile.cpp:2199:5
  

[clang] 83bde93 - resolve typo in the manual.

[via cfe-commits]

Author: Shivam
Date: 2022-04-01T03:15:17+05:30
New Revision: 83bde93cef369f85ea0adc25a4cf2349ea65bb3a

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

LOG: resolve typo in the manual.

Added: 


Modified: 
clang/www/analyzer/checker_dev_manual.html

Removed: 




diff  --git a/clang/www/analyzer/checker_dev_manual.html 
b/clang/www/analyzer/checker_dev_manual.html
index 7b63efefe11a9..20b4f41765a84 100644
--- a/clang/www/analyzer/checker_dev_manual.html
+++ b/clang/www/analyzer/checker_dev_manual.html
@@ -814,7 +814,7 @@ Additional Sources of 
Information
 https://llvm.org/doxygen;>LLVM doxygen, when dealing with classes
 from LLVM.
 
-  The https://discourse.llvm.org/c/clang/;> Clang Frontent Discourse 
site.
+  The https://discourse.llvm.org/c/clang/;> Clang Frontend Discourse 
site.
   This is the primary forum discussing ideas and posting questions about Clang 
development.
   For posting Clang Static Analyzer specific questions, please visit the
   https://discourse.llvm.org/c/clang/static-analyzer/;> Static 
Analyzer subcategory



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

[PATCH] D122699: [HLSL] Add Semantic syntax, and SV_GroupIndex

[Chris Bieneman via Phabricator via cfe-commits]

beanz updated this revision to Diff 419570.
beanz added a comment.

Updates based on feedback from @aaron.ballman & @MaskRay.

Thank you!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122699

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/AttributeCommonInfo.h
  clang/include/clang/Basic/Attributes.h
  clang/include/clang/Basic/DiagnosticParseKinds.td
  clang/include/clang/Parse/Parser.h
  clang/lib/Parse/CMakeLists.txt
  clang/lib/Parse/ParseDecl.cpp
  clang/lib/Parse/ParseHLSL.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/SemaHLSL/Semantics/entry_parameter.hlsl
  clang/test/SemaHLSL/Semantics/semantic_parsing.hlsl
  clang/utils/TableGen/ClangAttrEmitter.cpp

Index: clang/utils/TableGen/ClangAttrEmitter.cpp
===
--- clang/utils/TableGen/ClangAttrEmitter.cpp
+++ clang/utils/TableGen/ClangAttrEmitter.cpp
@@ -1493,6 +1493,9 @@
 Spelling += Namespace;
 Spelling += " ";
   }
+} else if (Variety == "HLSLSemantic") {
+  Prefix = ":";
+  Suffix = "";
 } else {
   llvm_unreachable("Unknown attribute syntax variety!");
 }
@@ -3300,7 +3303,7 @@
   // Separate all of the attributes out into four group: generic, C++11, GNU,
   // and declspecs. Then generate a big switch statement for each of them.
   std::vector Attrs = Records.getAllDerivedDefinitions("Attr");
-  std::vector Declspec, Microsoft, GNU, Pragma;
+  std::vector Declspec, Microsoft, GNU, Pragma, HLSLSemantic;
   std::map> CXX, C2x;
 
   // Walk over the list of all attributes, and split them out based on the
@@ -3321,6 +3324,8 @@
 C2x[SI.nameSpace()].push_back(R);
   else if (Variety == "Pragma")
 Pragma.push_back(R);
+  else if (Variety == "HLSLSemantic")
+HLSLSemantic.push_back(R);
 }
   }
 
@@ -3338,6 +3343,9 @@
   OS << "case AttrSyntax::Pragma:\n";
   OS << "  return llvm::StringSwitch(Name)\n";
   GenerateHasAttrSpellingStringSwitch(Pragma, OS, "Pragma");
+  OS << "case AttrSyntax::HLSLSemantic:\n";
+  OS << "  return llvm::StringSwitch(Name)\n";
+  GenerateHasAttrSpellingStringSwitch(HLSLSemantic, OS, "HLSLSemantic");
   auto fn = [](const char *Spelling, const char *Variety,
   const std::map> ) {
 OS << "case AttrSyntax::" << Variety << ": {\n";
@@ -4286,7 +4294,7 @@
 
   std::vector Attrs = Records.getAllDerivedDefinitions("Attr");
   std::vector GNU, Declspec, Microsoft, CXX11,
-  Keywords, Pragma, C2x;
+  Keywords, Pragma, C2x, HLSLSemantic;
   std::set Seen;
   for (const auto *A : Attrs) {
 const Record  = *A;
@@ -4338,6 +4346,8 @@
   Matches = 
 else if (Variety == "Pragma")
   Matches = 
+else if (Variety == "HLSLSemantic")
+  Matches = 
 
 assert(Matches && "Unsupported spelling variety found");
 
@@ -4373,6 +4383,8 @@
   StringMatcher("Name", Keywords, OS).Emit();
   OS << "  } else if (AttributeCommonInfo::AS_Pragma == Syntax) {\n";
   StringMatcher("Name", Pragma, OS).Emit();
+  OS << "  } else if (AttributeCommonInfo::AS_HLSLSemantic == Syntax) {\n";
+  StringMatcher("Name", HLSLSemantic, OS).Emit();
   OS << "  }\n";
   OS << "  return AttributeCommonInfo::UnknownAttribute;\n"
  << "}\n";
@@ -4482,7 +4494,7 @@
   }
 }
 
-enum class SpellingKind {
+enum class SpellingKind : size_t {
   GNU,
   CXX11,
   C2x,
@@ -4490,8 +4502,10 @@
   Microsoft,
   Keyword,
   Pragma,
+  HLSLSemantic,
+  NumSpellingKinds
 };
-static const size_t NumSpellingKinds = (size_t)SpellingKind::Pragma + 1;
+static const size_t NumSpellingKinds = (size_t)SpellingKind::NumSpellingKinds;
 
 class SpellingList {
   std::vector Spellings[NumSpellingKinds];
@@ -4509,7 +4523,8 @@
 .Case("Declspec", SpellingKind::Declspec)
 .Case("Microsoft", SpellingKind::Microsoft)
 .Case("Keyword", SpellingKind::Keyword)
-.Case("Pragma", SpellingKind::Pragma);
+.Case("Pragma", SpellingKind::Pragma)
+.Case("HLSLSemantic", SpellingKind::HLSLSemantic);
 std::string Name;
 if (!Spelling.nameSpace().empty()) {
   switch (Kind) {
@@ -4610,8 +4625,8 @@
   // List what spelling syntaxes the attribute supports.
   OS << ".. csv-table:: Supported Syntaxes\n";
   OS << "   :header: \"GNU\", \"C++11\", \"C2x\", \"``__declspec``\",";
-  OS << " \"Keyword\", \"``#pragma``\", \"``#pragma clang attribute``\"\n\n";
-  OS << "   \"";
+  OS << " \"Keyword\", \"``#pragma``\", \"``#pragma clang attribute``\",";
+  OS << " \"HLSL Semantic\"\n\n   \"";
   for (size_t Kind = 0; Kind != NumSpellingKinds; ++Kind) {
 SpellingKind K = (SpellingKind)Kind;
 // TODO: List Microsoft (IDL-style attribute) spellings once we fully
Index: 

[PATCH] D122760: [OpenMP] Add OpenMP variant extension to keep the unmangled name

[Joseph Huber via Phabricator via cfe-commits]

jhuber6 updated this revision to Diff 419567.
jhuber6 added a comment.
Herald added a reviewer: aaron.ballman.

Updating to use an attribute instead to get around the loss of the identifier 
info when using PCH. Unfortunately this still causes problems when using PCH.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122760

Files:
  clang/include/clang/Basic/Attr.td
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/Parse/ParseOpenMP.cpp
  clang/lib/Sema/SemaOpenMP.cpp
  clang/test/OpenMP/nvptx_declare_variant_name_mangling.cpp
  llvm/include/llvm/Frontend/OpenMP/OMPKinds.def

Index: llvm/include/llvm/Frontend/OpenMP/OMPKinds.def
===
--- llvm/include/llvm/Frontend/OpenMP/OMPKinds.def
+++ llvm/include/llvm/Frontend/OpenMP/OMPKinds.def
@@ -1157,6 +1157,7 @@
 __OMP_TRAIT_PROPERTY(implementation, extension, match_none)
 __OMP_TRAIT_PROPERTY(implementation, extension, disable_implicit_base)
 __OMP_TRAIT_PROPERTY(implementation, extension, allow_templates)
+__OMP_TRAIT_PROPERTY(implementation, extension, keep_original_name)
 
 __OMP_TRAIT_SET(user)
 
Index: clang/test/OpenMP/nvptx_declare_variant_name_mangling.cpp
===
--- clang/test/OpenMP/nvptx_declare_variant_name_mangling.cpp
+++ clang/test/OpenMP/nvptx_declare_variant_name_mangling.cpp
@@ -1,13 +1,15 @@
 // RUN: %clang_cc1 -verify -fopenmp -x c++ -triple powerpc64le-unknown-unknown -fopenmp-targets=nvptx64-nvidia-cuda -emit-llvm-bc %s -o %t-ppc-host.bc
-// RUN: %clang_cc1 -verify -fopenmp -x c++ -triple nvptx64-unknown-unknown -aux-triple powerpc64le-unknown-unknown -emit-llvm %s -fopenmp-is-device -fopenmp-host-ir-file-path %t-ppc-host.bc -o - | FileCheck %s --implicit-check-not='call i32 {@_Z3bazv|@_Z3barv}'
+// RUN: %clang_cc1 -verify -fopenmp -x c++ -triple nvptx64-unknown-unknown -aux-triple powerpc64le-unknown-unknown -emit-llvm %s -fopenmp-is-device -fopenmp-host-ir-file-path %t-ppc-host.bc -o - | FileCheck %s --implicit-check-not='call i32 {@_Z3bazv|@_Z3barv|@_ZL3foov}'
 // RUN: %clang_cc1 -verify -fopenmp -x c++ -triple nvptx64-unknown-unknown -aux-triple powerpc64le-unknown-unknown -emit-llvm %s -fopenmp-is-device -fopenmp-host-ir-file-path %t-ppc-host.bc -emit-pch -o %t
-// RUN: %clang_cc1 -verify -fopenmp -x c++ -triple nvptx64-unknown-unknown -aux-triple powerpc64le-unknown-unknown -emit-llvm %s -fopenmp-is-device -fopenmp-host-ir-file-path %t-ppc-host.bc -include-pch %t -o - | FileCheck %s --implicit-check-not='call i32 {@_Z3bazv|@_Z3barv}'
+// RUN: %clang_cc1 -verify -fopenmp -x c++ -triple nvptx64-unknown-unknown -aux-triple powerpc64le-unknown-unknown -emit-llvm %s -fopenmp-is-device -fopenmp-host-ir-file-path %t-ppc-host.bc -include-pch %t -o - | FileCheck %s --implicit-check-not='call i32 {@_Z3bazv|@_Z3barv|@_ZL3foov}'
 // expected-no-diagnostics
 
 // CHECK-DAG: @_Z3barv
 // CHECK-DAG: @_Z3bazv
+// CHECK-DAG: call noundef i32 @_ZL3foov()
 // CHECK-DAG: define{{.*}} @"_Z53bar$ompvariant$S2$s7$Pnvptx$Pnvptx64$S3$s9$Pmatch_anyv"
 // CHECK-DAG: define{{.*}} @"_Z53baz$ompvariant$S2$s7$Pnvptx$Pnvptx64$S3$s9$Pmatch_anyv"
+// CHECK-DAG: define{{.*}} @_ZL3foov
 // CHECK-DAG: call noundef i32 @"_Z53bar$ompvariant$S2$s7$Pnvptx$Pnvptx64$S3$s9$Pmatch_anyv"()
 // CHECK-DAG: call noundef i32 @"_Z53baz$ompvariant$S2$s7$Pnvptx$Pnvptx64$S3$s9$Pmatch_anyv"()
 
@@ -20,6 +22,8 @@
 
 int baz() { return 5; }
 
+static int foo() { return 3; }
+
 #pragma omp begin declare variant match(device = {arch(nvptx, nvptx64)}, implementation = {extension(match_any)})
 
 int bar() { return 2; }
@@ -28,13 +32,19 @@
 
 #pragma omp end declare variant
 
+#pragma omp begin declare variant match(device = {arch(nvptx, nvptx64)}, implementation = {extension(match_any, keep_original_name)})
+
+static int foo() { return 4; }
+
+#pragma omp end declare variant
+
 #pragma omp end declare target
 
 int main() {
   int res;
 #pragma omp target map(from \
: res)
-  res = bar() + baz();
+  res = bar() + baz() + foo();
   return res;
 }
 
Index: clang/lib/Sema/SemaOpenMP.cpp
===
--- clang/lib/Sema/SemaOpenMP.cpp
+++ clang/lib/Sema/SemaOpenMP.cpp
@@ -6910,6 +6910,11 @@
   /*AppendArgs=*/nullptr, /*AppendArgsSize=*/0);
   for (FunctionDecl *BaseFD : Bases)
 BaseFD->addAttr(OMPDeclareVariantA);
+  if (DVScope.TI->isExtensionActive(
+  llvm::omp::TraitProperty::
+  implementation_extension_keep_original_name))
+D->addAttr(OMPDeclareVariantNoMangleAttr::CreateImplicit(Context,
+  Bases.front()));
 }
 
 ExprResult Sema::ActOnOpenMPCall(ExprResult Call, Scope *Scope,
Index: clang/lib/Parse/ParseOpenMP.cpp
===
--- clang/lib/Parse/ParseOpenMP.cpp
+++ clang/lib/Parse/ParseOpenMP.cpp
@@ -957,6 +957,10 

[PATCH] D122846: [CUDA] Don't call inferCUDATargetForImplicitSpecialMember too early.

[Artem Belevich via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGfe528e721633: [CUDA] Dont call 
inferCUDATargetForImplicitSpecialMember too early. (authored by tra).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122846

Files:
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/test/SemaCUDA/pr54537.cu

Index: clang/test/SemaCUDA/pr54537.cu
===
--- /dev/null
+++ clang/test/SemaCUDA/pr54537.cu
@@ -0,0 +1,16 @@
+// Regression test for the crash in
+// https://github.com/llvm/llvm-project/issues/54537
+//
+// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify %s
+// expected-no-diagnostics
+
+template< class T > inline constexpr bool test_v = true;
+
+template 
+struct A {
+A(const T = 1 ) requires test_v;
+};
+
+struct B :  A {
+using A::A;
+};
Index: clang/lib/Sema/SemaDeclCXX.cpp
===
--- clang/lib/Sema/SemaDeclCXX.cpp
+++ clang/lib/Sema/SemaDeclCXX.cpp
@@ -13415,14 +13415,13 @@
   DefaultCon->setAccess(AS_public);
   DefaultCon->setDefaulted();
 
-  if (getLangOpts().CUDA) {
+  setupImplicitSpecialMemberType(DefaultCon, Context.VoidTy, None);
+
+  if (getLangOpts().CUDA)
 inferCUDATargetForImplicitSpecialMember(ClassDecl, CXXDefaultConstructor,
 DefaultCon,
 /* ConstRHS */ false,
 /* Diagnose */ false);
-  }
-
-  setupImplicitSpecialMemberType(DefaultCon, Context.VoidTy, None);
 
   // We don't need to use SpecialMemberIsTrivial here; triviality for default
   // constructors is easy to compute.
@@ -13696,14 +13695,13 @@
   Destructor->setAccess(AS_public);
   Destructor->setDefaulted();
 
-  if (getLangOpts().CUDA) {
+  setupImplicitSpecialMemberType(Destructor, Context.VoidTy, None);
+
+  if (getLangOpts().CUDA)
 inferCUDATargetForImplicitSpecialMember(ClassDecl, CXXDestructor,
 Destructor,
 /* ConstRHS */ false,
 /* Diagnose */ false);
-  }
-
-  setupImplicitSpecialMemberType(Destructor, Context.VoidTy, None);
 
   // We don't need to use SpecialMemberIsTrivial here; triviality for
   // destructors is easy to compute.
@@ -14336,14 +14334,13 @@
   CopyAssignment->setDefaulted();
   CopyAssignment->setImplicit();
 
-  if (getLangOpts().CUDA) {
+  setupImplicitSpecialMemberType(CopyAssignment, RetType, ArgType);
+
+  if (getLangOpts().CUDA)
 inferCUDATargetForImplicitSpecialMember(ClassDecl, CXXCopyAssignment,
 CopyAssignment,
 /* ConstRHS */ Const,
 /* Diagnose */ false);
-  }
-
-  setupImplicitSpecialMemberType(CopyAssignment, RetType, ArgType);
 
   // Add the parameter to the operator.
   ParmVarDecl *FromParam = ParmVarDecl::Create(Context, CopyAssignment,
@@ -14671,14 +14668,13 @@
   MoveAssignment->setDefaulted();
   MoveAssignment->setImplicit();
 
-  if (getLangOpts().CUDA) {
+  setupImplicitSpecialMemberType(MoveAssignment, RetType, ArgType);
+
+  if (getLangOpts().CUDA)
 inferCUDATargetForImplicitSpecialMember(ClassDecl, CXXMoveAssignment,
 MoveAssignment,
 /* ConstRHS */ false,
 /* Diagnose */ false);
-  }
-
-  setupImplicitSpecialMemberType(MoveAssignment, RetType, ArgType);
 
   // Add the parameter to the operator.
   ParmVarDecl *FromParam = ParmVarDecl::Create(Context, MoveAssignment,
@@ -15050,14 +15046,13 @@
   CopyConstructor->setAccess(AS_public);
   CopyConstructor->setDefaulted();
 
-  if (getLangOpts().CUDA) {
+  setupImplicitSpecialMemberType(CopyConstructor, Context.VoidTy, ArgType);
+
+  if (getLangOpts().CUDA)
 inferCUDATargetForImplicitSpecialMember(ClassDecl, CXXCopyConstructor,
 CopyConstructor,
 /* ConstRHS */ Const,
 /* Diagnose */ false);
-  }
-
-  setupImplicitSpecialMemberType(CopyConstructor, Context.VoidTy, ArgType);
 
   // During template instantiation of special member functions we need a
   // reliable TypeSourceInfo for the parameter types in order to allow functions
@@ -15190,14 +15185,13 @@
   MoveConstructor->setAccess(AS_public);
   MoveConstructor->setDefaulted();
 
-  if (getLangOpts().CUDA) {
+  setupImplicitSpecialMemberType(MoveConstructor, Context.VoidTy, ArgType);
+
+  if (getLangOpts().CUDA)
 inferCUDATargetForImplicitSpecialMember(ClassDecl, CXXMoveConstructor,
 

[clang] fe528e7 - [CUDA] Don't call inferCUDATargetForImplicitSpecialMember too early.

[Artem Belevich via cfe-commits]

Author: Artem Belevich
Date: 2022-03-31T13:49:12-07:00
New Revision: fe528e72163371e10242f4748dab687eef30a1f9

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

LOG: [CUDA] Don't call inferCUDATargetForImplicitSpecialMember too early.

Otherwise we may crash because the special member has not been sufficiently set
up yet. Fixes https://github.com/llvm/llvm-project/issues/54537

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

Added: 
clang/test/SemaCUDA/pr54537.cu

Modified: 
clang/lib/Sema/SemaDeclCXX.cpp

Removed: 




diff  --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index 0f56e6024f332..b852b82f359d4 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -13415,14 +13415,13 @@ CXXConstructorDecl 
*Sema::DeclareImplicitDefaultConstructor(
   DefaultCon->setAccess(AS_public);
   DefaultCon->setDefaulted();
 
-  if (getLangOpts().CUDA) {
+  setupImplicitSpecialMemberType(DefaultCon, Context.VoidTy, None);
+
+  if (getLangOpts().CUDA)
 inferCUDATargetForImplicitSpecialMember(ClassDecl, CXXDefaultConstructor,
 DefaultCon,
 /* ConstRHS */ false,
 /* Diagnose */ false);
-  }
-
-  setupImplicitSpecialMemberType(DefaultCon, Context.VoidTy, None);
 
   // We don't need to use SpecialMemberIsTrivial here; triviality for default
   // constructors is easy to compute.
@@ -13696,14 +13695,13 @@ CXXDestructorDecl 
*Sema::DeclareImplicitDestructor(CXXRecordDecl *ClassDecl) {
   Destructor->setAccess(AS_public);
   Destructor->setDefaulted();
 
-  if (getLangOpts().CUDA) {
+  setupImplicitSpecialMemberType(Destructor, Context.VoidTy, None);
+
+  if (getLangOpts().CUDA)
 inferCUDATargetForImplicitSpecialMember(ClassDecl, CXXDestructor,
 Destructor,
 /* ConstRHS */ false,
 /* Diagnose */ false);
-  }
-
-  setupImplicitSpecialMemberType(Destructor, Context.VoidTy, None);
 
   // We don't need to use SpecialMemberIsTrivial here; triviality for
   // destructors is easy to compute.
@@ -14336,14 +14334,13 @@ CXXMethodDecl 
*Sema::DeclareImplicitCopyAssignment(CXXRecordDecl *ClassDecl) {
   CopyAssignment->setDefaulted();
   CopyAssignment->setImplicit();
 
-  if (getLangOpts().CUDA) {
+  setupImplicitSpecialMemberType(CopyAssignment, RetType, ArgType);
+
+  if (getLangOpts().CUDA)
 inferCUDATargetForImplicitSpecialMember(ClassDecl, CXXCopyAssignment,
 CopyAssignment,
 /* ConstRHS */ Const,
 /* Diagnose */ false);
-  }
-
-  setupImplicitSpecialMemberType(CopyAssignment, RetType, ArgType);
 
   // Add the parameter to the operator.
   ParmVarDecl *FromParam = ParmVarDecl::Create(Context, CopyAssignment,
@@ -14671,14 +14668,13 @@ CXXMethodDecl 
*Sema::DeclareImplicitMoveAssignment(CXXRecordDecl *ClassDecl) {
   MoveAssignment->setDefaulted();
   MoveAssignment->setImplicit();
 
-  if (getLangOpts().CUDA) {
+  setupImplicitSpecialMemberType(MoveAssignment, RetType, ArgType);
+
+  if (getLangOpts().CUDA)
 inferCUDATargetForImplicitSpecialMember(ClassDecl, CXXMoveAssignment,
 MoveAssignment,
 /* ConstRHS */ false,
 /* Diagnose */ false);
-  }
-
-  setupImplicitSpecialMemberType(MoveAssignment, RetType, ArgType);
 
   // Add the parameter to the operator.
   ParmVarDecl *FromParam = ParmVarDecl::Create(Context, MoveAssignment,
@@ -15050,14 +15046,13 @@ CXXConstructorDecl 
*Sema::DeclareImplicitCopyConstructor(
   CopyConstructor->setAccess(AS_public);
   CopyConstructor->setDefaulted();
 
-  if (getLangOpts().CUDA) {
+  setupImplicitSpecialMemberType(CopyConstructor, Context.VoidTy, ArgType);
+
+  if (getLangOpts().CUDA)
 inferCUDATargetForImplicitSpecialMember(ClassDecl, CXXCopyConstructor,
 CopyConstructor,
 /* ConstRHS */ Const,
 /* Diagnose */ false);
-  }
-
-  setupImplicitSpecialMemberType(CopyConstructor, Context.VoidTy, ArgType);
 
   // During template instantiation of special member functions we need a
   // reliable TypeSourceInfo for the parameter types in order to allow 
functions
@@ -15190,14 +15185,13 @@ CXXConstructorDecl 
*Sema::DeclareImplicitMoveConstructor(
   MoveConstructor->setAccess(AS_public);
   MoveConstructor->setDefaulted();
 
-  if 

[PATCH] D122841: [analyzer] Consider all addrspaces in null dereference check

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

steakhal added a comment.

Couls you please provide some context about why did we have this suppression in 
the first place?

Other than that, I'm on board with this, but we should really investigate the 
historical reasons why we had this.

If it turns out to be a valid usecase, but a rather special one we could still 
have an analyzer config option making this change optional.

@NoQ do you have any background on this?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122841

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

[PATCH] D122820: [GH54588]Fix ItaniumMangler for NTTP unnamed unions w/ unnamed structs

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.



Comment at: clang/lib/AST/ItaniumMangle.cpp:5576
+  "program to refer to the anonymous union, and there is therefore no need 
"
+  "to mangle its name. '");
+}

erichkeane wrote:
> rjmccall wrote:
> > Unfortunately, I think class value mangling has a counter-example to this: 
> > you can have an anonymous struct or union which doesn't declare any named 
> > members but will of course nonetheless show up in the list of members in 
> > the enclosing class.  Code can even give it a non-zero initializer using 
> > list-initialization; that value can never be read, but it's there, and 
> > presumably it's part of uniquely determining a different template argument 
> > value and therefore needs to be mangled.
> > 
> > I don't know what we should do about this in the ABI, but we shouldn't 
> > crash in the compiler.
> I'm not really understanding what you mean?  Can you provide an example?  All 
> of the cases I could come up with like this ended up getting caught by the 
> 'zero -init' case.
> 
> That said, I considered this 'unreachable' to be a distinct improvement over 
> our current behavior which is to crash at effectively line 5558. 
> 
> Would you prefer some sort of 'fatal-error' emit here?  
I think the test case would be something like:

```
struct A {
  union { unsigned: 10; };
  int x;
  constexpr A(int x) : x(x) {}
};

template  void foo() {}
template void foo();
```

But also consider:

```
struct B {
  struct Nested {
union { unsigned: 10; };
int x;
  } nested;
  constexpr B(int x) : nested{5, x} {}
};

template  void bar() {}
template void bar();
```


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

https://reviews.llvm.org/D122820

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

[PATCH] D122846: [CUDA] Don't call inferCUDATargetForImplicitSpecialMember too early.

[Yaxun Liu via Phabricator via cfe-commits]

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

LGTM. Thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122846

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

[PATCH] D122838: [clang][dataflow] Add support for correlation of boolean (tracked) values

[Gábor Horváth via Phabricator via cfe-commits]

xazax.hun added inline comments.



Comment at: clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp:79
+for (BoolValue *Constraint : Env1.getFlowConditionConstraints()) {
+  Expr1 = (*Expr1, *Constraint);
+}

Hmm, interesting.
I think we view every boolean formula at a certain program point implicitly as 
`FlowConditionAtThatPoint && Formula`. And the flow condition at a program 
point should already be a disjunction of its predecessors.

So it would be interpreted as: `(FlowConditionPred1 || FlowConditionPred2) && 
(FormulaAtPred1 || FormulaAtPred2)`.
While this is great, this is not the strongest condition we could derive. 
`(FlowConditionPred1 && FormulaAtPred1)  || (FormulaAtPred2 && 
FlowConditionPred2)` created by this code snippet is stronger which is great.

My main concern is whether we would end up seeing an exponential explosion in 
the size of these formulas in the number of branches following each other in a 
sequence.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122838

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

[PATCH] D121176: [ASTStructuralEquivalence] Add support for comparing ObjCCategoryDecl.

[Volodymyr Sapsai via Phabricator via cfe-commits]

vsapsai added a comment.

Ping.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121176

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

[PATCH] D122822: [Clang][CodeGen]Add constant array support for __builtin_dump_sturct

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D122822#3420162 , @rsmith wrote:

> I'm also concerned that this builtin is making a lot of design decisions on 
> behalf of the programmer, meaning that either it does exactly what you want 
> or it's not suitable for your use and there's not much you can do about it.

FWIW, I'm not concerned by this. My understanding of this builtin is that it 
exists solely as a developer debugging aid (https://reviews.llvm.org/D44093) 
predominately for kernel folks when a debugger may not be available. As such, I 
view this the same as our other `dump()` methods -- we try to dump useful 
information, but ultimately, nobody should be relying on this output or 
building anything around it.

> A different design that let the programmer choose whether to recurse into 
> structs and arrays and similarly let the programmer choose how to format the 
> fields would seem preferable, but I'm not confident there's a good way to 
> expose that in C (though in C++ there seem to be good designs to choose 
> from). As an example of how that manifests here, the choice to print each 
> element as a separate line for a struct member of type `const char str[256];` 
> is probably going to make the output very unreadable, but choosing to print 
> every array of `char` using `"%s"` will be equally bad for arrays that don't 
> contain text -- the "right" answer for a dumping facility probably involves 
> checking whether the array contains only printable characters and trimming a 
> trailing sequence of zeroes, but that seems like vastly more complexity than 
> we'd want in code generated by a builtin.

While this is interesting, I am not certain there's a lot of value to it as a 
debugging aid for times when a debugger is not available to you. It sounds like 
a fair amount of complexity, so I'd like to know whether there's a strong need 
for this much generality.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122822

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

[PATCH] D122820: [GH54588]Fix ItaniumMangler for NTTP unnamed unions w/ unnamed structs

[Erich Keane via Phabricator via cfe-commits]

erichkeane updated this revision to Diff 419548.
erichkeane added a comment.

Handle the unnamed bitfield case.

I'm still not sure how to reproduce the 'unreachable' example I added that 
@rjmccall suggested was possible, nor how we can deal with that, so waiting on 
his feedback for how he thinks we should handle that case.

Presumably I could replace it with a 'don't know how to handle that yet' fatal 
error like we do in the MS Mangler?


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

https://reviews.llvm.org/D122820

Files:
  clang/lib/AST/ItaniumMangle.cpp
  clang/test/CodeGenCXX/mangle-nttp-anon-union.cpp

Index: clang/test/CodeGenCXX/mangle-nttp-anon-union.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/mangle-nttp-anon-union.cpp
@@ -0,0 +1,113 @@
+// RUN: %clang_cc1 -std=c++20 -emit-llvm %s -o - -triple=x86_64-linux-gnu | FileCheck %s
+// RUN: %clang_cc1 -std=c++20 -emit-llvm %s -o - -triple=x86_64-linux-gnu | llvm-cxxfilt | FileCheck %s --check-prefix DEMANGLED
+
+template
+struct wrapper1 {
+  union {
+struct {
+  T RightName;
+};
+  };
+};
+
+template
+struct wrapper2 {
+  union {
+struct {
+  T RightName;
+};
+T WrongName;
+  };
+};
+
+struct Base {
+  int WrongName;
+};
+
+template 
+struct wrapper3 {
+  union {
+struct : Base {
+  T RightName; };
+T WrongName;
+  };
+};
+
+template 
+struct wrapper4 {
+  union {
+int RightName;
+struct {
+  T WrongName;
+};
+T AlsoWrongName;
+  };
+};
+
+template 
+struct wrapper5 {
+  union {
+struct {
+  struct {
+T RightName;
+  };
+  T WrongName;
+};
+  };
+};
+
+template
+struct wrapper6 {
+  union {
+union{
+int : 5;
+T RightName;
+};
+  };
+};
+
+
+
+template void dummy(){}
+
+
+void uses() {
+  // Zero init'ed cases.
+  dummy{}>();
+  // CHECK: call void @_Z5dummyIXtl8wrapper1Iivv
+  // DEMANGLED: call void @void dummy{}>()()
+  dummy{}>();
+  // CHECK: call void @_Z5dummyIXtl8wrapper2Ifvv
+  // DEMANGLED: call void @void dummy{}>()()
+  dummy{}>();
+  // CHECK: call void @_Z5dummyIXtl8wrapper3Isvv
+  // DEMANGLED: call void @void dummy{}>()()
+  dummy{}>();
+  // CHECK: call void @_Z5dummyIXtl8wrapper4Idvv
+  // DEMANGLED: call void @void dummy{}>()()
+  dummy{}>();
+  // CHECK: call void @_Z5dummyIXtl8wrapper5Ixvv
+  // DEMANGLED: call void @void dummy{}>()()
+  dummy{}>();
+  // CHECK: call void @_Z5dummyIXtl8wrapper6Iivv
+  // DEMANGLED: call void @void dummy{}>()()
+
+  dummy{123.0}>();
+  // CHECK: call void @_Z5dummyIXtl8wrapper1IdEtlNS1_Ut_Edi9RightNametlNS2_Ut_ELd405ec000EEvv
+  // DEMANGLED: call void @void dummy{wrapper1::'unnamed'{.RightName = wrapper1::'unnamed'::'unnamed'{0x1.ecp+6}}}>()()
+  dummy{123.0}>();
+  // CHECK: call void @_Z5dummyIXtl8wrapper2IdEtlNS1_Ut_Edi9RightNametlNS2_Ut_ELd405ec000EEvv
+  // DEMANGLED: call void @void dummy{wrapper2::'unnamed'{.RightName = wrapper2::'unnamed'::'unnamed'{0x1.ecp+6}}}>()()
+  dummy{123, 456}>();
+  // CHECK: call void @_Z5dummyIXtl8wrapper3IdEtlNS1_Ut_Edi9RightNametlNS2_Ut_Etl4BaseLi123EELd407c8000EEvv
+  // DEMANGLED: call void @void dummy{wrapper3::'unnamed'{.RightName = wrapper3::'unnamed'::'unnamed'{Base{123}, 0x1.c8p+8}}}>()()
+  dummy{123}>();
+  // CHECK: call void @_Z5dummyIXtl8wrapper4IdEtlNS1_Ut_Edi9RightNameLi123Evv
+  // DEMANGLED: call void @void dummy{wrapper4::'unnamed'{.RightName = 123}}>()()
+  dummy{123.0, 456.0}>();
+  // CHECK: call void @_Z5dummyIXtl8wrapper5IdEtlNS1_Ut_Edi9RightNametlNS2_Ut_EtlNS3_Ut_ELd405ec000EELd407c8000EEvv
+  // DEMANGLED: call void @void dummy{wrapper5::'unnamed'{.RightName = wrapper5::'unnamed'::'unnamed'{wrapper5::'unnamed'::'unnamed'::'unnamed'{0x1.ecp+6}, 0x1.c8p+8}}}>()()
+  dummy{1}>();
+  // CHECK: call void @_Z5dummyIXtl8wrapper6IdEtlNS1_Ut_Edi9RightNametlNS2_Ut_Edi9RightNameLd3ff0EEvv
+  // DEMANGELD: call void @void dummy{wrapper6::'unnamed'{.RightName = wrapper6::'unnamed'::'unnamed'{.RightName = 0x1p+0}}}>()()   
+}
Index: clang/lib/AST/ItaniumMangle.cpp
===
--- clang/lib/AST/ItaniumMangle.cpp
+++ clang/lib/AST/ItaniumMangle.cpp
@@ -5545,6 +5545,41 @@
   return T;
 }
 
+static IdentifierInfo *getUnionInitName(const FieldDecl *FD) {
+  // According to:
+  // http://itanium-cxx-abi.github.io/cxx-abi/abi.html#mangling.anonymous
+  // For the purposes of mangling, the name of an anonymous union is considered
+  // to be the name of the first named data member found by a pre-order,
+  // depth-first, declaration-order walk of the data members of the anonymous
+  // union.
+
+  if (FD->getIdentifier())
+return FD->getIdentifier();
+
+  // The only cases where the identifer of a FieldDecl would be blank is if the
+  // field represents an anonymous record type or if it is an unnamed 

[PATCH] D122822: [Clang][CodeGen]Add constant array support for __builtin_dump_sturct

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

In D122822#3420357 , @rsmith wrote:

> One thing we could do would be to build a descriptor for the type (as data) 
> -- a type name string, a type "kind", plus (for a record type) a list of 
> (field type descriptor, field name, offset) tuples -- and pass it to a 
> user-specified function. We could provide a library implementation of a 
> dumping function that takes that descriptor and a function with printf's 
> signature and provides the "normal" formatting per the current 
> `__builtin_dump_struct` behavior -- and indeed we could then implement 
> `__builtin_dump_struct(p, f)` as a call to 
> `normal_dumping_function(__builtin_type_descriptor(T), p, f)`. I'm not sure 
> how comfortable we'd be putting that library implementation in compiler-rt 
> versus somewhere else, though it presumably wouldn't be very large.

This seems to be quickly decaying into 'implement Reflection', doesn't it?  I 
see this `__builtin_dump_struct` to be a useful debugging builtin.  As much as 
I see 'Reflection' as incredibly useful, I would think this is intended to be 
something much simpler, right?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122822

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

[PATCH] D122822: [Clang][CodeGen]Add constant array support for __builtin_dump_sturct

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

rsmith added a comment.

One thing we could do would be to build a descriptor for the type (as data) -- 
a type name string, a type "kind", plus (for a record type) a list of (field 
type descriptor, field name, offset) tuples -- and pass it to a user-specified 
function. We could provide a library implementation of a dumping function that 
takes that descriptor and a function with printf's signature and provides the 
"normal" formatting per the current `__builtin_dump_struct` behavior -- and 
indeed we could then implement `__builtin_dump_struct(p, f)` as a call to 
`normal_dumping_function(__builtin_type_descriptor(T), p, f)`. I'm not sure how 
comfortable we'd be putting that library implementation in compiler-rt versus 
somewhere else, though it presumably wouldn't be very large.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122822

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

[PATCH] D122846: [CUDA] Don't call inferCUDATargetForImplicitSpecialMember too early.

[Artem Belevich via Phabricator via cfe-commits]

tra created this revision.
tra added a reviewer: yaxunl.
Herald added a subscriber: bixia.
Herald added a project: All.
tra requested review of this revision.
Herald added a project: clang.

Otherwise we may crash because the special member has not been sufficiently set 
up yet. 
Fixes #54537


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D122846

Files:
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/test/SemaCUDA/pr54537.cu

Index: clang/test/SemaCUDA/pr54537.cu
===
--- /dev/null
+++ clang/test/SemaCUDA/pr54537.cu
@@ -0,0 +1,16 @@
+// Regression test for the crash in
+// https://github.com/llvm/llvm-project/issues/54537
+//
+// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify %s
+// expected-no-diagnostics
+
+template< class T > inline constexpr bool test_v = true;
+
+template 
+struct A {
+A(const T = 1 ) requires test_v;
+};
+
+struct B :  A {
+using A::A;
+};
Index: clang/lib/Sema/SemaDeclCXX.cpp
===
--- clang/lib/Sema/SemaDeclCXX.cpp
+++ clang/lib/Sema/SemaDeclCXX.cpp
@@ -13415,14 +13415,13 @@
   DefaultCon->setAccess(AS_public);
   DefaultCon->setDefaulted();
 
-  if (getLangOpts().CUDA) {
+  setupImplicitSpecialMemberType(DefaultCon, Context.VoidTy, None);
+
+  if (getLangOpts().CUDA)
 inferCUDATargetForImplicitSpecialMember(ClassDecl, CXXDefaultConstructor,
 DefaultCon,
 /* ConstRHS */ false,
 /* Diagnose */ false);
-  }
-
-  setupImplicitSpecialMemberType(DefaultCon, Context.VoidTy, None);
 
   // We don't need to use SpecialMemberIsTrivial here; triviality for default
   // constructors is easy to compute.
@@ -13696,14 +13695,13 @@
   Destructor->setAccess(AS_public);
   Destructor->setDefaulted();
 
-  if (getLangOpts().CUDA) {
+  setupImplicitSpecialMemberType(Destructor, Context.VoidTy, None);
+
+  if (getLangOpts().CUDA)
 inferCUDATargetForImplicitSpecialMember(ClassDecl, CXXDestructor,
 Destructor,
 /* ConstRHS */ false,
 /* Diagnose */ false);
-  }
-
-  setupImplicitSpecialMemberType(Destructor, Context.VoidTy, None);
 
   // We don't need to use SpecialMemberIsTrivial here; triviality for
   // destructors is easy to compute.
@@ -14336,14 +14334,13 @@
   CopyAssignment->setDefaulted();
   CopyAssignment->setImplicit();
 
-  if (getLangOpts().CUDA) {
+  setupImplicitSpecialMemberType(CopyAssignment, RetType, ArgType);
+
+  if (getLangOpts().CUDA)
 inferCUDATargetForImplicitSpecialMember(ClassDecl, CXXCopyAssignment,
 CopyAssignment,
 /* ConstRHS */ Const,
 /* Diagnose */ false);
-  }
-
-  setupImplicitSpecialMemberType(CopyAssignment, RetType, ArgType);
 
   // Add the parameter to the operator.
   ParmVarDecl *FromParam = ParmVarDecl::Create(Context, CopyAssignment,
@@ -14671,14 +14668,13 @@
   MoveAssignment->setDefaulted();
   MoveAssignment->setImplicit();
 
-  if (getLangOpts().CUDA) {
+  setupImplicitSpecialMemberType(MoveAssignment, RetType, ArgType);
+
+  if (getLangOpts().CUDA)
 inferCUDATargetForImplicitSpecialMember(ClassDecl, CXXMoveAssignment,
 MoveAssignment,
 /* ConstRHS */ false,
 /* Diagnose */ false);
-  }
-
-  setupImplicitSpecialMemberType(MoveAssignment, RetType, ArgType);
 
   // Add the parameter to the operator.
   ParmVarDecl *FromParam = ParmVarDecl::Create(Context, MoveAssignment,
@@ -15050,14 +15046,13 @@
   CopyConstructor->setAccess(AS_public);
   CopyConstructor->setDefaulted();
 
-  if (getLangOpts().CUDA) {
+  setupImplicitSpecialMemberType(CopyConstructor, Context.VoidTy, ArgType);
+
+  if (getLangOpts().CUDA)
 inferCUDATargetForImplicitSpecialMember(ClassDecl, CXXCopyConstructor,
 CopyConstructor,
 /* ConstRHS */ Const,
 /* Diagnose */ false);
-  }
-
-  setupImplicitSpecialMemberType(CopyConstructor, Context.VoidTy, ArgType);
 
   // During template instantiation of special member functions we need a
   // reliable TypeSourceInfo for the parameter types in order to allow functions
@@ -15190,14 +15185,13 @@
   MoveConstructor->setAccess(AS_public);
   MoveConstructor->setDefaulted();
 
-  if (getLangOpts().CUDA) {
+  setupImplicitSpecialMemberType(MoveConstructor, Context.VoidTy, ArgType);
+
+  if (getLangOpts().CUDA)
 inferCUDATargetForImplicitSpecialMember(ClassDecl, CXXMoveConstructor,
 

[PATCH] D122766: [clang] Use forward slash as the path separator for Windows in __FILE__, __builtin_FILE(), and std::source_location

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo added a comment.

> When targeting Windows, the path separator used when targeting Windows 
> depends on the build environment when Clang _itself_ is built. This leads to 
> inconsistencies in Chrome builds where Clang running on non-Windows 
> environments uses the forward slash (/) path separator while Clang running on 
> Windows builds uses the backslash (\) path separator. To fix this, we make 
> Clang use forward slashes whenever it is targeting Windows.

IMO, this problem formulation is a bit too narrow/specific - you'd have the 
same phenomenon if you'd cross compile things on Windows, targeting e.g. Linux. 
So I think the condition for normalizing paths would be if the build host OS is 
Windows, not based on the target OS. (I.e. in the current patch form, the 
normalization that is chosen, when targeting Windows from Unix, is a no-op 
anyway.)

Then secondly - if the source paths are relative paths, making them OS-agnostic 
in this way does make sense. But if they are absolute, they are pretty much by 
definition specific to the build host, and can't be expected to make sense on a 
different platform. So for such a case, there's less reason to try to normalize 
the paths. But as Windows path handling in most cases does tolerate paths with 
forward slashes, I guess it's ok to do anyway.

Overall, I think this is a sensible thing to do. I'm not entirely sure if an 
option for the behaviour is needed or not though - I guess that's the main 
question.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122766

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

[PATCH] D122820: [GH54588]Fix ItaniumMangler for NTTP unnamed unions w/ unnamed structs

[Erich Keane via Phabricator via cfe-commits]

erichkeane added inline comments.



Comment at: clang/lib/AST/ItaniumMangle.cpp:5561
+  // field represents an anonymous record type.
+  const CXXRecordDecl *RD = FD->getType()->getAsCXXRecordDecl();
+

rjmccall wrote:
> Alas, bit-fields are also allowed to be anonymous.
Ah, good point, i'll update the comment and add a test.



Comment at: clang/lib/AST/ItaniumMangle.cpp:5576
+  "program to refer to the anonymous union, and there is therefore no need 
"
+  "to mangle its name. '");
+}

rjmccall wrote:
> Unfortunately, I think class value mangling has a counter-example to this: 
> you can have an anonymous struct or union which doesn't declare any named 
> members but will of course nonetheless show up in the list of members in the 
> enclosing class.  Code can even give it a non-zero initializer using 
> list-initialization; that value can never be read, but it's there, and 
> presumably it's part of uniquely determining a different template argument 
> value and therefore needs to be mangled.
> 
> I don't know what we should do about this in the ABI, but we shouldn't crash 
> in the compiler.
I'm not really understanding what you mean?  Can you provide an example?  All 
of the cases I could come up with like this ended up getting caught by the 
'zero -init' case.

That said, I considered this 'unreachable' to be a distinct improvement over 
our current behavior which is to crash at effectively line 5558. 

Would you prefer some sort of 'fatal-error' emit here?  


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

https://reviews.llvm.org/D122820

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

[PATCH] D122822: [Clang][CodeGen]Add constant array support for __builtin_dump_sturct

[Erich Keane via Phabricator via cfe-commits]

erichkeane added inline comments.



Comment at: clang/lib/CodeGen/CGBuiltin.cpp:2048
+  QualType Type) {
+  static llvm::DenseMap Types;
   if (Types.empty()) {

yihanaa wrote:
> erichkeane wrote:
> > yihanaa wrote:
> > > erichkeane wrote:
> > > > yihanaa wrote:
> > > > > yihanaa wrote:
> > > > > > erichkeane wrote:
> > > > > > > yihanaa wrote:
> > > > > > > > erichkeane wrote:
> > > > > > > > > yihanaa wrote:
> > > > > > > > > > erichkeane wrote:
> > > > > > > > > > > Instead of this initialization, can we make this a 
> > > > > > > > > > > constexpr constructor/collection of some sort and 
> > > > > > > > > > > instantiate it inline?  
> > > > > > > > > > > Instead of this initialization, can we make this a 
> > > > > > > > > > > constexpr constructor/collection of some sort and 
> > > > > > > > > > > instantiate it inline?  
> > > > > > > > > > 
> > > > > > > > > > I don't have a good idea because it relies on Context to 
> > > > > > > > > > get some types like const char *
> > > > > > > > > We could at minimum do a in inline-initializer instead of the 
> > > > > > > > > branch below.
> > > > > > > > I still haven't come up with a good idea to do it
> > > > > > > Hrmph, `DenseMap` doesn't seem to have an init list ctor?!
> > > > > > > 
> > > > > > > Since this is a finite, known at compile time list of low 'N', 
> > > > > > > I'd suggest just making it `static std::array > > > > > > StringRef>> Types = {{Context.CharTy, "%c"},...`.
> > > > > > > 
> > > > > > > Then just doing a llvm::find_if.  The list is small enough I 
> > > > > > > suspect the DenseMap overhead is more than the use of 
> > > > > > > `llvm::find_if`.
> > > > > > Haha,DenseMap also has no constexpr ctor. I think the time 
> > > > > > complexity of these two is the same. 
> > > > > The Context.CharTy... etc. are not static, So we might need a static 
> > > > > value instead of it, and compare it with QualType
> > > > They don't have to be static?  This should do 'magic static' 
> > > > initialization.
> > > I can't agree more. it might be a type identifier, such as 
> > > getAsString()'s return value
> > Actually, I don't think the collection can be static!  When we re-use the 
> > same process for multiple TUs, we get a new ASTContext.  SO this would 
> > result in us having types that don't necessarily 'match'.  I think this 
> > collection/checks need to either be stored in ASTContext, or in an 
> > 'if-else' tree here.
> I also don't think it should be maintained in this place. Shouldn't this be 
> better maintained together with the printf parameter checking part, what do 
> you think?
I'm unfamiliar with what you mean.

HOWEVER, the comments by @rsmith and @rjmccall I think are blocking on this 
feature/functionality anyway.  So we might still wish to refactor this builtin 
significantly based on what they come up with.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122822

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

[PATCH] D122820: [GH54588]Fix ItaniumMangler for NTTP unnamed unions w/ unnamed structs

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

Unfortunately, this initialization case does allow a situation where




Comment at: clang/lib/AST/ItaniumMangle.cpp:5561
+  // field represents an anonymous record type.
+  const CXXRecordDecl *RD = FD->getType()->getAsCXXRecordDecl();
+

Alas, bit-fields are also allowed to be anonymous.



Comment at: clang/lib/AST/ItaniumMangle.cpp:5576
+  "program to refer to the anonymous union, and there is therefore no need 
"
+  "to mangle its name. '");
+}

Unfortunately, I think class value mangling has a counter-example to this: you 
can have an anonymous struct or union which doesn't declare any named members 
but will of course nonetheless show up in the list of members in the enclosing 
class.  Code can even give it a non-zero initializer using list-initialization; 
that value can never be read, but it's there, and presumably it's part of 
uniquely determining a different template argument value and therefore needs to 
be mangled.

I don't know what we should do about this in the ABI, but we shouldn't crash in 
the compiler.


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

https://reviews.llvm.org/D122820

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

[PATCH] D122822: [Clang][CodeGen]Add constant array support for __builtin_dump_sturct

[Wang Yihan via Phabricator via cfe-commits]

yihanaa added inline comments.



Comment at: clang/lib/CodeGen/CGBuiltin.cpp:2048
+  QualType Type) {
+  static llvm::DenseMap Types;
   if (Types.empty()) {

erichkeane wrote:
> yihanaa wrote:
> > erichkeane wrote:
> > > yihanaa wrote:
> > > > yihanaa wrote:
> > > > > erichkeane wrote:
> > > > > > yihanaa wrote:
> > > > > > > erichkeane wrote:
> > > > > > > > yihanaa wrote:
> > > > > > > > > erichkeane wrote:
> > > > > > > > > > Instead of this initialization, can we make this a 
> > > > > > > > > > constexpr constructor/collection of some sort and 
> > > > > > > > > > instantiate it inline?  
> > > > > > > > > > Instead of this initialization, can we make this a 
> > > > > > > > > > constexpr constructor/collection of some sort and 
> > > > > > > > > > instantiate it inline?  
> > > > > > > > > 
> > > > > > > > > I don't have a good idea because it relies on Context to get 
> > > > > > > > > some types like const char *
> > > > > > > > We could at minimum do a in inline-initializer instead of the 
> > > > > > > > branch below.
> > > > > > > I still haven't come up with a good idea to do it
> > > > > > Hrmph, `DenseMap` doesn't seem to have an init list ctor?!
> > > > > > 
> > > > > > Since this is a finite, known at compile time list of low 'N', I'd 
> > > > > > suggest just making it `static std::array > > > > > StringRef>> Types = {{Context.CharTy, "%c"},...`.
> > > > > > 
> > > > > > Then just doing a llvm::find_if.  The list is small enough I 
> > > > > > suspect the DenseMap overhead is more than the use of 
> > > > > > `llvm::find_if`.
> > > > > Haha,DenseMap also has no constexpr ctor. I think the time complexity 
> > > > > of these two is the same. 
> > > > The Context.CharTy... etc. are not static, So we might need a static 
> > > > value instead of it, and compare it with QualType
> > > They don't have to be static?  This should do 'magic static' 
> > > initialization.
> > I can't agree more. it might be a type identifier, such as getAsString()'s 
> > return value
> Actually, I don't think the collection can be static!  When we re-use the 
> same process for multiple TUs, we get a new ASTContext.  SO this would result 
> in us having types that don't necessarily 'match'.  I think this 
> collection/checks need to either be stored in ASTContext, or in an 'if-else' 
> tree here.
I also don't think it should be maintained in this place. Shouldn't this be 
better maintained together with the printf parameter checking part, what do you 
think?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122822

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

[PATCH] D122822: [Clang][CodeGen]Add constant array support for __builtin_dump_sturct

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

In D122822#3420162 , @rsmith wrote:

> I'm concerned about the direction of this patch given @rjmccall's comments on 
> https://reviews.llvm.org/D112626 -- presumably the way we'd want to address 
> those comments would be to convert a `__builtin_dump_struct(a, f)` call into 
> a single `f("...",  a.x, a.y, a.z)` call in Sema,  and that approach doesn't 
> seem compatible with generating code to loop over array elements.

Well, we could generate a whole helper function that takes the struct by 
reference.  That would also drastically improve the code-size impact of this 
builtin, since it's likely that the builtin will be used multiple times with 
the same struct.

> I'm also concerned that this builtin is making a lot of design decisions on 
> behalf of the programmer, meaning that either it does exactly what you want 
> or it's not suitable for your use and there's not much you can do about it.

Yeah, I agree with this point, too; it's a very opinionated builtin.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122822

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

[PATCH] D122822: [Clang][CodeGen]Add constant array support for __builtin_dump_sturct

[Wang Yihan via Phabricator via cfe-commits]

yihanaa added a comment.

In D122822#3420162 , @rsmith wrote:

> I'm concerned about the direction of this patch given @rjmccall's comments on 
> https://reviews.llvm.org/D112626 -- presumably the way we'd want to address 
> those comments would be to convert a `__builtin_dump_struct(a, f)` call into 
> a single `f("...",  a.x, a.y, a.z)` call in Sema,  and that approach doesn't 
> seem compatible with generating code to loop over array elements.
>
> I'm also concerned that this builtin is making a lot of design decisions on 
> behalf of the programmer, meaning that either it does exactly what you want 
> or it's not suitable for your use and there's not much you can do about it. A 
> different design that let the programmer choose whether to recurse into 
> structs and arrays and similarly let the programmer choose how to format the 
> fields would seem preferable, but I'm not confident there's a good way to 
> expose that in C (though in C++ there seem to be good designs to choose 
> from). As an example of how that manifests here, the choice to print each 
> element as a separate line for a struct member of type `const char str[256];` 
> is probably going to make the output very unreadable, but choosing to print 
> every array of `char` using `"%s"` will be equally bad for arrays that don't 
> contain text -- the "right" answer for a dumping facility probably involves 
> checking whether the array contains only printable characters and trimming a 
> trailing sequence of zeroes, but that seems like vastly more complexity than 
> we'd want in code generated by a builtin.

I'm also a newbie and only gradually integrated into the community with the 
help of @erichkeane , @aaron.ballman and others many times, tthis problem may 
be a bit difficult for me at the moment, but I'd be like to do a bigger 
overhaul of the whole builtin if that would make it more great.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122822

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

[PATCH] D122822: [Clang][CodeGen]Add constant array support for __builtin_dump_sturct

[Erich Keane via Phabricator via cfe-commits]

erichkeane added inline comments.



Comment at: clang/lib/CodeGen/CGBuiltin.cpp:2048
+  QualType Type) {
+  static llvm::DenseMap Types;
   if (Types.empty()) {

yihanaa wrote:
> erichkeane wrote:
> > yihanaa wrote:
> > > yihanaa wrote:
> > > > erichkeane wrote:
> > > > > yihanaa wrote:
> > > > > > erichkeane wrote:
> > > > > > > yihanaa wrote:
> > > > > > > > erichkeane wrote:
> > > > > > > > > Instead of this initialization, can we make this a constexpr 
> > > > > > > > > constructor/collection of some sort and instantiate it 
> > > > > > > > > inline?  
> > > > > > > > > Instead of this initialization, can we make this a constexpr 
> > > > > > > > > constructor/collection of some sort and instantiate it 
> > > > > > > > > inline?  
> > > > > > > > 
> > > > > > > > I don't have a good idea because it relies on Context to get 
> > > > > > > > some types like const char *
> > > > > > > We could at minimum do a in inline-initializer instead of the 
> > > > > > > branch below.
> > > > > > I still haven't come up with a good idea to do it
> > > > > Hrmph, `DenseMap` doesn't seem to have an init list ctor?!
> > > > > 
> > > > > Since this is a finite, known at compile time list of low 'N', I'd 
> > > > > suggest just making it `static std::array> 
> > > > > Types = {{Context.CharTy, "%c"},...`.
> > > > > 
> > > > > Then just doing a llvm::find_if.  The list is small enough I suspect 
> > > > > the DenseMap overhead is more than the use of `llvm::find_if`.
> > > > Haha,DenseMap also has no constexpr ctor. I think the time complexity 
> > > > of these two is the same. 
> > > The Context.CharTy... etc. are not static, So we might need a static 
> > > value instead of it, and compare it with QualType
> > They don't have to be static?  This should do 'magic static' initialization.
> I can't agree more. it might be a type identifier, such as getAsString()'s 
> return value
Actually, I don't think the collection can be static!  When we re-use the same 
process for multiple TUs, we get a new ASTContext.  SO this would result in us 
having types that don't necessarily 'match'.  I think this collection/checks 
need to either be stored in ASTContext, or in an 'if-else' tree here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122822

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

[PATCH] D122608: Fix behavior of ifuncs with 'used' extern "C" static functions

[Erich Keane via Phabricator via cfe-commits]

erichkeane updated this revision to Diff 419530.
erichkeane added a comment.

fix a typo in the 'triple' .


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

https://reviews.llvm.org/D122608

Files:
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/CodeGen/CodeGenModule.h
  clang/test/CodeGenCXX/externc-ifunc-resolver.cpp
  clang/test/SemaCXX/externc-ifunc-resolver.cpp

Index: clang/test/SemaCXX/externc-ifunc-resolver.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/externc-ifunc-resolver.cpp
@@ -0,0 +1,16 @@
+// RUN: %clang_cc1 -emit-llvm-only -triple x86_64-linux-gnu -verify %s
+
+extern "C" {
+__attribute__((used)) static void *resolve_foo() { return 0; }
+namespace NS {
+__attribute__((used)) static void *resolve_foo() { return 0; }
+} // namespace NS
+
+// FIXME: This diagnostic is pretty confusing, the issue is that the existence
+// of the two functions suppresses the 'alias' creation, and thus the ifunc
+// resolution via the alias as well. In the future we should probably find
+// some way to improve this diagnostic (likely by diagnosing when we decide
+// this case suppresses alias creation).
+__attribute__((ifunc("resolve_foo"))) void foo(); // expected-error{{ifunc must point to a defined function}}
+}
+
Index: clang/test/CodeGenCXX/externc-ifunc-resolver.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/externc-ifunc-resolver.cpp
@@ -0,0 +1,16 @@
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -o - %s | FileCheck %s
+
+extern "C" {
+__attribute__((used)) static void *resolve_foo() { return 0; }
+__attribute__((ifunc("resolve_foo"))) char *foo();
+__attribute__((ifunc("resolve_foo"))) void foo2(int);
+__attribute__((ifunc("resolve_foo"))) char foo3(float);
+__attribute__((ifunc("resolve_foo"))) char foo4(float);
+}
+
+// CHECK: @resolve_foo = internal alias i8* (), i8* ()* @_ZL11resolve_foov
+// CHECK: @foo = ifunc i8* (), bitcast (i8* ()* @_ZL11resolve_foov to i8* ()* ()*)
+// CHECK: @foo2 = ifunc void (i32), bitcast (i8* ()* @_ZL11resolve_foov to void (i32)* ()*)
+// CHECK: @foo3 = ifunc i8 (float), bitcast (i8* ()* @_ZL11resolve_foov to i8 (float)* ()*)
+// CHECK: @foo4 = ifunc i8 (float), bitcast (i8* ()* @_ZL11resolve_foov to i8 (float)* ()*)
+// CHECK: define internal noundef i8* @_ZL11resolve_foov()
Index: clang/lib/CodeGen/CodeGenModule.h
===
--- clang/lib/CodeGen/CodeGenModule.h
+++ clang/lib/CodeGen/CodeGenModule.h
@@ -1570,6 +1570,12 @@
   /// Emit the link options introduced by imported modules.
   void EmitModuleLinkOptions();
 
+  /// Helper function for EmitStaticExternCAliases that clears the uses of
+  /// 'Elem' if it is used exclusively by ifunc resolvers. Returns 'true' if it
+  /// was successful erases Elem.
+  bool CheckAndReplaceExternCIFuncs(llvm::GlobalValue *Elem,
+llvm::GlobalValue *CppFunc);
+
   /// Emit aliases for internal-linkage declarations inside "C" language
   /// linkage specifications, giving them the "expected" name where possible.
   void EmitStaticExternCAliases();
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -507,7 +507,6 @@
   EmitVTablesOpportunistically();
   applyGlobalValReplacements();
   applyReplacements();
-  checkAliases();
   emitMultiVersionFunctions();
   EmitCXXGlobalInitFunc();
   EmitCXXGlobalCleanUpFunc();
@@ -539,6 +538,7 @@
   EmitCtorList(GlobalDtors, "llvm.global_dtors");
   EmitGlobalAnnotations();
   EmitStaticExternCAliases();
+  checkAliases();
   EmitDeferredUnusedCoverageMappings();
   CodeGenPGO(*this).setValueProfilingFlag(getModule());
   if (CoverageMapping)
@@ -6315,6 +6315,67 @@
   GlobalMetadata->addOperand(llvm::MDNode::get(CGM.getLLVMContext(), Ops));
 }
 
+bool CodeGenModule::CheckAndReplaceExternCIFuncs(llvm::GlobalValue *Elem,
+ llvm::GlobalValue *CppFunc) {
+  // Store the list of ifuncs we need to replace uses in.
+  llvm::SmallVector IFuncs;
+  // List of ConstantExprs that we should be able to delete when we're done
+  // here.
+  llvm::SmallVector CEs;
+
+  // First make sure that all users of this are ifuncs (or ifuncs via a
+  // bitcast), and collect the list of ifuncs and CEs so we can work on them
+  // later.
+  for (llvm::User *User : Elem->users()) {
+// Users can either be a bitcast ConstExpr that is used by the ifuncs, OR an
+// ifunc directly. In any other case, just give up, as we don't know what we
+// could break by changing those.
+if (auto *ConstExpr = dyn_cast(User)) {
+  if (ConstExpr->getOpcode() != llvm::Instruction::BitCast)
+return false;
+
+  for (llvm::User *CEUser : ConstExpr->users()) {
+if (auto *IFunc = 

[PATCH] D122608: Fix behavior of ifuncs with 'used' extern "C" static functions

[Erich Keane via Phabricator via cfe-commits]

erichkeane updated this revision to Diff 419526.
erichkeane marked an inline comment as done.
erichkeane added a comment.

Applied clang-format, removed 'Name' as Aaron found is no longer used, fixed 
the Sema test to not run on Windows triple, where ifunc is not available


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

https://reviews.llvm.org/D122608

Files:
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/CodeGen/CodeGenModule.h
  clang/test/CodeGenCXX/externc-ifunc-resolver.cpp
  clang/test/SemaCXX/externc-ifunc-resolver.cpp

Index: clang/test/SemaCXX/externc-ifunc-resolver.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/externc-ifunc-resolver.cpp
@@ -0,0 +1,16 @@
+// RUN: %clang_cc1 -emit-llvm-only -triple x86-64-linux-gnu -verify %s
+
+extern "C" {
+__attribute__((used)) static void *resolve_foo() { return 0; }
+namespace NS {
+__attribute__((used)) static void *resolve_foo() { return 0; }
+} // namespace NS
+
+// FIXME: This diagnostic is pretty confusing, the issue is that the existence
+// of the two functions suppresses the 'alias' creation, and thus the ifunc
+// resolution via the alias as well. In the future we should probably find
+// some way to improve this diagnostic (likely by diagnosing when we decide
+// this case suppresses alias creation).
+__attribute__((ifunc("resolve_foo"))) void foo(); // expected-error{{ifunc must point to a defined function}}
+}
+
Index: clang/test/CodeGenCXX/externc-ifunc-resolver.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/externc-ifunc-resolver.cpp
@@ -0,0 +1,16 @@
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -o - %s | FileCheck %s
+
+extern "C" {
+__attribute__((used)) static void *resolve_foo() { return 0; }
+__attribute__((ifunc("resolve_foo"))) char *foo();
+__attribute__((ifunc("resolve_foo"))) void foo2(int);
+__attribute__((ifunc("resolve_foo"))) char foo3(float);
+__attribute__((ifunc("resolve_foo"))) char foo4(float);
+}
+
+// CHECK: @resolve_foo = internal alias i8* (), i8* ()* @_ZL11resolve_foov
+// CHECK: @foo = ifunc i8* (), bitcast (i8* ()* @_ZL11resolve_foov to i8* ()* ()*)
+// CHECK: @foo2 = ifunc void (i32), bitcast (i8* ()* @_ZL11resolve_foov to void (i32)* ()*)
+// CHECK: @foo3 = ifunc i8 (float), bitcast (i8* ()* @_ZL11resolve_foov to i8 (float)* ()*)
+// CHECK: @foo4 = ifunc i8 (float), bitcast (i8* ()* @_ZL11resolve_foov to i8 (float)* ()*)
+// CHECK: define internal noundef i8* @_ZL11resolve_foov()
Index: clang/lib/CodeGen/CodeGenModule.h
===
--- clang/lib/CodeGen/CodeGenModule.h
+++ clang/lib/CodeGen/CodeGenModule.h
@@ -1570,6 +1570,12 @@
   /// Emit the link options introduced by imported modules.
   void EmitModuleLinkOptions();
 
+  /// Helper function for EmitStaticExternCAliases that clears the uses of
+  /// 'Elem' if it is used exclusively by ifunc resolvers. Returns 'true' if it
+  /// was successful erases Elem.
+  bool CheckAndReplaceExternCIFuncs(llvm::GlobalValue *Elem,
+llvm::GlobalValue *CppFunc);
+
   /// Emit aliases for internal-linkage declarations inside "C" language
   /// linkage specifications, giving them the "expected" name where possible.
   void EmitStaticExternCAliases();
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -507,7 +507,6 @@
   EmitVTablesOpportunistically();
   applyGlobalValReplacements();
   applyReplacements();
-  checkAliases();
   emitMultiVersionFunctions();
   EmitCXXGlobalInitFunc();
   EmitCXXGlobalCleanUpFunc();
@@ -539,6 +538,7 @@
   EmitCtorList(GlobalDtors, "llvm.global_dtors");
   EmitGlobalAnnotations();
   EmitStaticExternCAliases();
+  checkAliases();
   EmitDeferredUnusedCoverageMappings();
   CodeGenPGO(*this).setValueProfilingFlag(getModule());
   if (CoverageMapping)
@@ -6315,6 +6315,67 @@
   GlobalMetadata->addOperand(llvm::MDNode::get(CGM.getLLVMContext(), Ops));
 }
 
+bool CodeGenModule::CheckAndReplaceExternCIFuncs(llvm::GlobalValue *Elem,
+ llvm::GlobalValue *CppFunc) {
+  // Store the list of ifuncs we need to replace uses in.
+  llvm::SmallVector IFuncs;
+  // List of ConstantExprs that we should be able to delete when we're done
+  // here.
+  llvm::SmallVector CEs;
+
+  // First make sure that all users of this are ifuncs (or ifuncs via a
+  // bitcast), and collect the list of ifuncs and CEs so we can work on them
+  // later.
+  for (llvm::User *User : Elem->users()) {
+// Users can either be a bitcast ConstExpr that is used by the ifuncs, OR an
+// ifunc directly. In any other case, just give up, as we don't know what we
+// could break by changing those.
+if (auto *ConstExpr = dyn_cast(User)) {
+ 

[PATCH] D122822: [Clang][CodeGen]Add constant array support for __builtin_dump_sturct

[Wang Yihan via Phabricator via cfe-commits]

yihanaa added inline comments.



Comment at: clang/lib/CodeGen/CGBuiltin.cpp:2048
+  QualType Type) {
+  static llvm::DenseMap Types;
   if (Types.empty()) {

erichkeane wrote:
> yihanaa wrote:
> > yihanaa wrote:
> > > erichkeane wrote:
> > > > yihanaa wrote:
> > > > > erichkeane wrote:
> > > > > > yihanaa wrote:
> > > > > > > erichkeane wrote:
> > > > > > > > Instead of this initialization, can we make this a constexpr 
> > > > > > > > constructor/collection of some sort and instantiate it inline?  
> > > > > > > > Instead of this initialization, can we make this a constexpr 
> > > > > > > > constructor/collection of some sort and instantiate it inline?  
> > > > > > > 
> > > > > > > I don't have a good idea because it relies on Context to get some 
> > > > > > > types like const char *
> > > > > > We could at minimum do a in inline-initializer instead of the 
> > > > > > branch below.
> > > > > I still haven't come up with a good idea to do it
> > > > Hrmph, `DenseMap` doesn't seem to have an init list ctor?!
> > > > 
> > > > Since this is a finite, known at compile time list of low 'N', I'd 
> > > > suggest just making it `static std::array> 
> > > > Types = {{Context.CharTy, "%c"},...`.
> > > > 
> > > > Then just doing a llvm::find_if.  The list is small enough I suspect 
> > > > the DenseMap overhead is more than the use of `llvm::find_if`.
> > > Haha,DenseMap also has no constexpr ctor. I think the time complexity of 
> > > these two is the same. 
> > The Context.CharTy... etc. are not static, So we might need a static value 
> > instead of it, and compare it with QualType
> They don't have to be static?  This should do 'magic static' initialization.
I can't agree more. it might be a type identifier, such as getAsString()'s 
return value


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122822

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

[PATCH] D122822: [Clang][CodeGen]Add constant array support for __builtin_dump_sturct

[Erich Keane via Phabricator via cfe-commits]

erichkeane added inline comments.



Comment at: clang/lib/CodeGen/CGBuiltin.cpp:2048
+  QualType Type) {
+  static llvm::DenseMap Types;
   if (Types.empty()) {

yihanaa wrote:
> yihanaa wrote:
> > erichkeane wrote:
> > > yihanaa wrote:
> > > > erichkeane wrote:
> > > > > yihanaa wrote:
> > > > > > erichkeane wrote:
> > > > > > > Instead of this initialization, can we make this a constexpr 
> > > > > > > constructor/collection of some sort and instantiate it inline?  
> > > > > > > Instead of this initialization, can we make this a constexpr 
> > > > > > > constructor/collection of some sort and instantiate it inline?  
> > > > > > 
> > > > > > I don't have a good idea because it relies on Context to get some 
> > > > > > types like const char *
> > > > > We could at minimum do a in inline-initializer instead of the branch 
> > > > > below.
> > > > I still haven't come up with a good idea to do it
> > > Hrmph, `DenseMap` doesn't seem to have an init list ctor?!
> > > 
> > > Since this is a finite, known at compile time list of low 'N', I'd 
> > > suggest just making it `static std::array> 
> > > Types = {{Context.CharTy, "%c"},...`.
> > > 
> > > Then just doing a llvm::find_if.  The list is small enough I suspect the 
> > > DenseMap overhead is more than the use of `llvm::find_if`.
> > Haha,DenseMap also has no constexpr ctor. I think the time complexity of 
> > these two is the same. 
> The Context.CharTy... etc. are not static, So we might need a static value 
> instead of it, and compare it with QualType
They don't have to be static?  This should do 'magic static' initialization.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122822

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

[PATCH] D122822: [Clang][CodeGen]Add constant array support for __builtin_dump_sturct

[Wang Yihan via Phabricator via cfe-commits]

yihanaa added inline comments.



Comment at: clang/lib/CodeGen/CGBuiltin.cpp:2048
+  QualType Type) {
+  static llvm::DenseMap Types;
   if (Types.empty()) {

yihanaa wrote:
> erichkeane wrote:
> > yihanaa wrote:
> > > erichkeane wrote:
> > > > yihanaa wrote:
> > > > > erichkeane wrote:
> > > > > > Instead of this initialization, can we make this a constexpr 
> > > > > > constructor/collection of some sort and instantiate it inline?  
> > > > > > Instead of this initialization, can we make this a constexpr 
> > > > > > constructor/collection of some sort and instantiate it inline?  
> > > > > 
> > > > > I don't have a good idea because it relies on Context to get some 
> > > > > types like const char *
> > > > We could at minimum do a in inline-initializer instead of the branch 
> > > > below.
> > > I still haven't come up with a good idea to do it
> > Hrmph, `DenseMap` doesn't seem to have an init list ctor?!
> > 
> > Since this is a finite, known at compile time list of low 'N', I'd suggest 
> > just making it `static std::array> Types = 
> > {{Context.CharTy, "%c"},...`.
> > 
> > Then just doing a llvm::find_if.  The list is small enough I suspect the 
> > DenseMap overhead is more than the use of `llvm::find_if`.
> Haha,DenseMap also has no constexpr ctor. I think the time complexity of 
> these two is the same. 
The Context.CharTy... etc. are not static, So we might need a static value 
instead of it, and compare it with QualType


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122822

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

[PATCH] D122608: Fix behavior of ifuncs with 'used' extern "C" static functions

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

FWIW, I'm seeing a precommit CI failure on Windows:

  Failed Tests (1):
Clang :: SemaCXX/externc-ifunc-resolver.cpp

May as well also fix up the clang-format issues in the review.




Comment at: clang/lib/CodeGen/CodeGenModule.cpp:6319
+bool CodeGenModule::CheckAndReplaceExternCIFuncs(
+llvm::GlobalValue *Elem, IdentifierInfo *Name,
+llvm::GlobalValue *CppFunc) {

`Name` appears to be entirely unused?


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

https://reviews.llvm.org/D122608

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

[PATCH] D122822: [Clang][CodeGen]Add constant array support for __builtin_dump_sturct

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

rsmith added subscribers: rjmccall, rsmith.
rsmith added a comment.

I'm concerned about the direction of this patch given @rjmccall's comments on 
https://reviews.llvm.org/D112626 -- presumably the way we'd want to address 
those comments would be to convert a `__builtin_dump_struct(a, f)` call into a 
single `f("...",  a.x, a.y, a.z)` call in Sema,  and that approach doesn't seem 
compatible with generating code to loop over array elements.

I'm also concerned that this builtin is making a lot of design decisions on 
behalf of the programmer, meaning that either it does exactly what you want or 
it's not suitable for your use and there's not much you can do about it. A 
different design that let the programmer choose whether to recurse into structs 
and arrays and similarly let the programmer choose how to format the fields 
would seem preferable, but I'm not confident there's a good way to expose that 
in C (though in C++ there seem to be good designs to choose from). As an 
example of how that manifests here, the choice to print each element as a 
separate line for a struct member of type `const char str[256];` is probably 
going to make the output very unreadable, but choosing to print every array of 
`char` using `"%s"` will be equally bad for arrays that don't contain text -- 
the "right" answer for a dumping facility probably involves checking whether 
the array contains only printable characters and trimming a trailing sequence 
of zeroes, but that seems like vastly more complexity than we'd want in code 
generated by a builtin.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122822

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

Re: [clang] 1905416 - [HLSL] Further improve to numthreads diagnostics

[Aaron Ballman via cfe-commits]

On Thu, Mar 31, 2022 at 12:34 PM Chris Bieneman via cfe-commits
 wrote:
>
>
> Author: Chris Bieneman
> Date: 2022-03-31T11:34:01-05:00
> New Revision: 19054163e11a6632b4973c936e5aa93ec742c866
>
> URL: 
> https://github.com/llvm/llvm-project/commit/19054163e11a6632b4973c936e5aa93ec742c866
> DIFF: 
> https://github.com/llvm/llvm-project/commit/19054163e11a6632b4973c936e5aa93ec742c866.diff
>
> LOG: [HLSL] Further improve to numthreads diagnostics
>
> This adds diagnostics for conflicting attributes on the same
> declarataion, conflicting attributes on a forward and final
> declaration, and defines a more narrowly scoped HLSLEntry attribute
> target.
>
> Big shout out to @aaron.ballman for the great feedback and review on
> this!

Thank you for the extra test coverage and fixes! One suggestion below.

> Added:
>
>
> Modified:
> clang/include/clang/Basic/Attr.td
> clang/include/clang/Basic/DiagnosticSemaKinds.td
> clang/include/clang/Sema/Sema.h
> clang/lib/Sema/SemaDecl.cpp
> clang/lib/Sema/SemaDeclAttr.cpp
> clang/test/SemaHLSL/num_threads.hlsl
>
> Removed:
>
>
>
> 
> diff  --git a/clang/include/clang/Basic/Attr.td 
> b/clang/include/clang/Basic/Attr.td
> index 97b1027742f60..4789493399ec2 100644
> --- a/clang/include/clang/Basic/Attr.td
> +++ b/clang/include/clang/Basic/Attr.td
> @@ -126,8 +126,10 @@ def FunctionTmpl
>   FunctionDecl::TK_FunctionTemplate}],
>  "function templates">;
>
> -def GlobalFunction
> -: SubsetSubjectisGlobal()}], "global functions">;
> +def HLSLEntry
> +: SubsetSubject +[{S->isExternallyVisible() && !isa(S)}],
> +"global functions">;
>
>  def ClassTmpl : SubsetSubjectgetDescribedClassTemplate()}],
>"class templates">;
> @@ -3946,7 +3948,7 @@ def Error : InheritableAttr {
>  def HLSLNumThreads: InheritableAttr {
>let Spellings = [Microsoft<"numthreads">];
>let Args = [IntArgument<"X">, IntArgument<"Y">, IntArgument<"Z">];
> -  let Subjects = SubjectList<[GlobalFunction]>;
> +  let Subjects = SubjectList<[HLSLEntry]>;
>let LangOpts = [HLSL];
>let Documentation = [NumThreadsDocs];
>  }
>
> diff  --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td 
> b/clang/include/clang/Basic/DiagnosticSemaKinds.td
> index a272cb741270f..aec172c39ed9a 100644
> --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
> +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
> @@ -11570,6 +11570,7 @@ def err_hlsl_attr_unsupported_in_stage : 
> Error<"attribute %0 is unsupported in %
>
>  def err_hlsl_numthreads_argument_oor : Error<"argument '%select{X|Y|Z}0' to 
> numthreads attribute cannot exceed %1">;
>  def err_hlsl_numthreads_invalid : Error<"total number of threads cannot 
> exceed %0">;
> +def err_hlsl_attribute_param_mismatch : Error<"%0 attribute parameters do 
> not match the previous declaration">;

I'd prefer if we reused warn_duplicate_attribute; I'd be fine if we
added an error variety of that as in:

def err_duplicate_attribute : Error;

This makes it easier for us to reuse the diagnostic for other
attributes and makes it a bit more discoverable because the name is
similar to the warning variant.

~Aaron

>
>  } // end of sema component.
>
>
> diff  --git a/clang/include/clang/Sema/Sema.h 
> b/clang/include/clang/Sema/Sema.h
> index 6523c3001c294..c0ad55d52bb31 100644
> --- a/clang/include/clang/Sema/Sema.h
> +++ b/clang/include/clang/Sema/Sema.h
> @@ -3471,6 +3471,9 @@ class Sema final {
>EnforceTCBLeafAttr *mergeEnforceTCBLeafAttr(Decl *D,
>const EnforceTCBLeafAttr );
>BTFDeclTagAttr *mergeBTFDeclTagAttr(Decl *D, const BTFDeclTagAttr );
> +  HLSLNumThreadsAttr *mergeHLSLNumThreadsAttr(Decl *D,
> +  const AttributeCommonInfo ,
> +  int X, int Y, int Z);
>
>void mergeDeclAttributes(NamedDecl *New, Decl *Old,
> AvailabilityMergeKind AMK = AMK_Redeclaration);
>
> diff  --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
> index b913f805bc877..1e25346fde6f6 100644
> --- a/clang/lib/Sema/SemaDecl.cpp
> +++ b/clang/lib/Sema/SemaDecl.cpp
> @@ -2770,6 +2770,9 @@ static bool mergeDeclAttribute(Sema , NamedDecl *D,
>  NewAttr = S.mergeEnforceTCBLeafAttr(D, *TCBLA);
>else if (const auto *BTFA = dyn_cast(Attr))
>  NewAttr = S.mergeBTFDeclTagAttr(D, *BTFA);
> +  else if (const auto *NT = dyn_cast(Attr))
> +NewAttr =
> +S.mergeHLSLNumThreadsAttr(D, *NT, NT->getX(), NT->getY(), 
> NT->getZ());
>else if (Attr->shouldInheritEvenIfAlreadyPresent() || !DeclHasAttr(D, 
> Attr))
>  NewAttr = cast(Attr->clone(S.Context));
>
>
> diff  --git a/clang/lib/Sema/SemaDeclAttr.cpp 
> b/clang/lib/Sema/SemaDeclAttr.cpp
> index 

[PATCH] D122822: [Clang][CodeGen]Add constant array support for __builtin_dump_sturct

[Wang Yihan via Phabricator via cfe-commits]

yihanaa added inline comments.



Comment at: clang/lib/CodeGen/CGBuiltin.cpp:2048
+  QualType Type) {
+  static llvm::DenseMap Types;
   if (Types.empty()) {

erichkeane wrote:
> yihanaa wrote:
> > erichkeane wrote:
> > > yihanaa wrote:
> > > > erichkeane wrote:
> > > > > Instead of this initialization, can we make this a constexpr 
> > > > > constructor/collection of some sort and instantiate it inline?  
> > > > > Instead of this initialization, can we make this a constexpr 
> > > > > constructor/collection of some sort and instantiate it inline?  
> > > > 
> > > > I don't have a good idea because it relies on Context to get some types 
> > > > like const char *
> > > We could at minimum do a in inline-initializer instead of the branch 
> > > below.
> > I still haven't come up with a good idea to do it
> Hrmph, `DenseMap` doesn't seem to have an init list ctor?!
> 
> Since this is a finite, known at compile time list of low 'N', I'd suggest 
> just making it `static std::array> Types = 
> {{Context.CharTy, "%c"},...`.
> 
> Then just doing a llvm::find_if.  The list is small enough I suspect the 
> DenseMap overhead is more than the use of `llvm::find_if`.
Haha,DenseMap also has no constexpr ctor. I think the time complexity of these 
two is the same. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122822

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

[PATCH] D122841: [analyzer] Consider all addrspaces in null dereference check

[Vince Bridgers via Phabricator via cfe-commits]

vabridgers created this revision.
vabridgers added reviewers: NoQ, steakhal, martong.
Herald added subscribers: manas, ASDenysPetrov, dkrupp, donat.nagy, Szelethus, 
mikhail.ramalho, a.sidorin, rnkovacs, szepet, baloghadamsoftware, xazax.hun.
Herald added a project: All.
vabridgers requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This change enables the null dereference checker to consider all null
dereferences, even those with address space qualifiers. The original
checker did not check null dereferences with address space qualifiers
for some reason. There's no reason to not consider all null
dereferences.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D122841

Files:
  clang/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp
  clang/test/Analysis/cast-value-notes.cpp
  clang/test/Analysis/null-deref-ps.c
  clang/test/Analysis/nullptr.cpp

Index: clang/test/Analysis/nullptr.cpp
===
--- clang/test/Analysis/nullptr.cpp
+++ clang/test/Analysis/nullptr.cpp
@@ -160,16 +160,19 @@
 public:
   int x;
   ~AS1() {
-int AS_ATTRIBUTE *x = 0;
-*x = 3; // no-warning
+int AS_ATTRIBUTE *x = 0; // expected-note{{'x' initialized to a null pointer value}}
+*x = 3;  // expected-warning{{Dereference of null pointer}}
+ // expected-note@-1{{Dereference of null pointer}}
   }
 };
 void test_address_space_field_access() {
-  AS1 AS_ATTRIBUTE *pa = 0;
-  pa->x = 0; // no-warning
+  AS1 AS_ATTRIBUTE *pa = 0; // expected-note{{'pa' initialized to a null pointer value}}
+  pa->x = 0;// expected-warning{{Access to field 'x' results in a dereference of a null pointer (loaded from variable 'pa')}}
+// expected-note@-1{{Access to field 'x' results in a dereference of a null pointer (loaded from variable 'pa')}}
 }
 void test_address_space_bind() {
-  AS1 AS_ATTRIBUTE *pa = 0;
-  AS1 AS_ATTRIBUTE  = *pa;
+  AS1 AS_ATTRIBUTE *pa = 0;  // expected-note{{'pa' initialized to a null pointer value}}
+  AS1 AS_ATTRIBUTE  = *pa; // expected-warning{{Dereference of null pointer}}
+ // expected-note@-1{{Dereference of null pointer}}
   r.x = 0; // no-warning
 }
Index: clang/test/Analysis/null-deref-ps.c
===
--- clang/test/Analysis/null-deref-ps.c
+++ clang/test/Analysis/null-deref-ps.c
@@ -315,17 +315,17 @@
 #define AS_ATTRIBUTE volatile __attribute__((address_space(256)))
 #define _get_base() ((void * AS_ATTRIBUTE *)0)
 void* test_address_space_array(unsigned long slot) {
-  return _get_base()[slot]; // no-warning
+  return _get_base()[slot]; // expected-warning{{Array access results in a null pointer dereference}}
 }
 void test_address_space_condition(int AS_ATTRIBUTE *cpu_data) {
if (cpu_data == 0) {
-*cpu_data = 3; // no-warning
+ *cpu_data = 3; // expected-warning{{Dereference of null pointer}}
   }
 }
 struct X { int member; };
 int test_address_space_member(void) {
   struct X AS_ATTRIBUTE *data = (struct X AS_ATTRIBUTE *)0UL;
   int ret;
-  ret = data->member; // no-warning
+  ret = data->member; // expected-warning{{Access to field 'member' results in a dereference of a null pointer}}
   return ret;
 }
Index: clang/test/Analysis/cast-value-notes.cpp
===
--- clang/test/Analysis/cast-value-notes.cpp
+++ clang/test/Analysis/cast-value-notes.cpp
@@ -1,6 +1,6 @@
-// RUN: %clang_analyze_cc1 -std=c++14 \
-// RUN:  -analyzer-checker=core,apiModeling.llvm.CastValue,debug.ExprInspection\
-// RUN:  -analyzer-output=text -verify -DDEFAULT_TRIPLE %s 2>&1 | FileCheck %s -check-prefix=DEFAULT-CHECK
+// R UN: %clang_analyze_cc1 -std=c++14 \
+// R UN:  -analyzer-checker=core,apiModeling.llvm.CastValue,debug.ExprInspection\
+// R UN:  -analyzer-output=text -verify -DDEFAULT_TRIPLE %s 2>&1 | FileCheck %s -check-prefix=DEFAULT-CHECK
 //
 // RUN: %clang_analyze_cc1 -std=c++14 -triple amdgcn-unknown-unknown \
 // RUN: -analyzer-checker=core,apiModeling.llvm.CastValue,debug.ExprInspection\
@@ -50,6 +50,9 @@
 #elif defined(AMDGCN_TRIPLE)
 void evalReferences(const Shape ) {
   const auto  = dyn_cast(S);
+  // expected-note@-1 {{Assuming 'S' is not a 'Circle'}}
+  // expected-note@-2 {{Dereference of null pointer}}
+  // expected-warning@-3 {{Dereference of null pointer}}
   clang_analyzer_printState();
   // AMDGCN-CHECK: "dynamic_types": [
   // AMDGCN-CHECK-NEXT: { "region": "SymRegion{reg_$0}", "dyn_type": "const __attribute__((address_space(3))) class clang::Circle &", "sub_classable": true }
Index: clang/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp
@@ -109,11 

[PATCH] D122822: [Clang][CodeGen]Add constant array support for __builtin_dump_sturct

[Erich Keane via Phabricator via cfe-commits]

erichkeane added inline comments.



Comment at: clang/lib/CodeGen/CGBuiltin.cpp:2048
+  QualType Type) {
+  static llvm::DenseMap Types;
   if (Types.empty()) {

yihanaa wrote:
> erichkeane wrote:
> > yihanaa wrote:
> > > erichkeane wrote:
> > > > Instead of this initialization, can we make this a constexpr 
> > > > constructor/collection of some sort and instantiate it inline?  
> > > > Instead of this initialization, can we make this a constexpr 
> > > > constructor/collection of some sort and instantiate it inline?  
> > > 
> > > I don't have a good idea because it relies on Context to get some types 
> > > like const char *
> > We could at minimum do a in inline-initializer instead of the branch below.
> I still haven't come up with a good idea to do it
Hrmph, `DenseMap` doesn't seem to have an init list ctor?!

Since this is a finite, known at compile time list of low 'N', I'd suggest just 
making it `static std::array> Types = 
{{Context.CharTy, "%c"},...`.

Then just doing a llvm::find_if.  The list is small enough I suspect the 
DenseMap overhead is more than the use of `llvm::find_if`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122822

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

[PATCH] D122805: [analyzer][ctu] Only import const and trivial VarDecls

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

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

In D122805#3419662 , @martong wrote:

> In D122805#3419129 , @steakhal 
> wrote:
>
>> Please add a test case where the class is trivial but the global variable of 
>> such is non-const, thus the initialized expression is not imported.
>
> I've added a new case, but that is not very meaningful, because 
> `clang-extdef-mapping` is not capable of emitting the USR for the non-const 
> variable.

What do you mean by //not capable//?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122805

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

[PATCH] D122822: [Clang][CodeGen]Add constant array support for __builtin_dump_sturct

[Wang Yihan via Phabricator via cfe-commits]

yihanaa added inline comments.



Comment at: clang/lib/CodeGen/CGBuiltin.cpp:2048
+  QualType Type) {
+  static llvm::DenseMap Types;
   if (Types.empty()) {

erichkeane wrote:
> yihanaa wrote:
> > erichkeane wrote:
> > > Instead of this initialization, can we make this a constexpr 
> > > constructor/collection of some sort and instantiate it inline?  
> > > Instead of this initialization, can we make this a constexpr 
> > > constructor/collection of some sort and instantiate it inline?  
> > 
> > I don't have a good idea because it relies on Context to get some types 
> > like const char *
> We could at minimum do a in inline-initializer instead of the branch below.
I still haven't come up with a good idea to do it


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122822

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

[PATCH] D122838: [clang][dataflow] Add support for correlation of boolean (tracked) values

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel created this revision.
ymandel added reviewers: xazax.hun, sgatev.
Herald added subscribers: tschuett, steakhal, rnkovacs.
Herald added a project: All.
ymandel requested review of this revision.
Herald added a project: clang.

This patch extends the join logic for environments to explicitly handle
boolean values. It creates the disjunction of both source values, guarded by the
respective flow conditions from each input environment. This change allows the
framework to reason about boolean correlations across multiple branches (and
subsequent joins).


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D122838

Files:
  clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
  clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
  clang/unittests/Analysis/FlowSensitive/TransferTest.cpp

Index: clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
@@ -35,6 +35,7 @@
 using ::testing::IsNull;
 using ::testing::NotNull;
 using ::testing::Pair;
+using ::testing::SizeIs;
 
 class TransferTest : public ::testing::Test {
 protected:
@@ -2410,4 +2411,58 @@
   });
 }
 
+TEST_F(TransferTest, CorrelatedBranches) {
+  std::string Code = R"(
+void target(bool B, bool C) {
+  if (B) {
+return;
+  }
+  (void)0;
+  /*[[p0]]*/
+  if (C) {
+B = true;
+/*[[p1]]*/
+  }
+  if (B) {
+(void)0;
+/*[[p2]]*/
+  }
+}
+  )";
+  runDataflow(
+  Code, [](llvm::ArrayRef<
+   std::pair>>
+   Results,
+   ASTContext ) {
+ASSERT_THAT(Results, SizeIs(3));
+
+const ValueDecl *CDecl = findValueDecl(ASTCtx, "C");
+ASSERT_THAT(CDecl, NotNull());
+
+{
+  ASSERT_THAT(Results[2], Pair("p0", _));
+  const Environment  = Results[2].second.Env;
+  const ValueDecl *BDecl = findValueDecl(ASTCtx, "B");
+  ASSERT_THAT(BDecl, NotNull());
+  auto  = *cast(Env.getValue(*BDecl, SkipPast::None));
+
+  EXPECT_TRUE(Env.flowConditionImplies(Env.makeNot(BVal)));
+}
+
+{
+  ASSERT_THAT(Results[1], Pair("p1", _));
+  const Environment  = Results[1].second.Env;
+  auto  = *cast(Env.getValue(*CDecl, SkipPast::None));
+  EXPECT_TRUE(Env.flowConditionImplies(CVal));
+}
+
+{
+  ASSERT_THAT(Results[0], Pair("p2", _));
+  const Environment  = Results[0].second.Env;
+  auto  = *cast(Env.getValue(*CDecl, SkipPast::None));
+  EXPECT_TRUE(Env.flowConditionImplies(CVal));
+}
+  });
+}
+
 } // namespace
Index: clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
===
--- clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
+++ clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
@@ -65,6 +65,35 @@
   return Model.compareEquivalent(Type, *Val1, Env1, *Val2, Env2);
 }
 
+/// Attempts to merge distinct values `Val1` and `Val1` in `Env1` and `Env2`,
+/// respectively, of the same type `Type`. Merging generally produces a single
+/// value that (soundly) approximates the two inputs, although the actual
+/// meaning depends on `Model`.
+static Value *mergeDistinctValues(QualType Type, Value *Val1, Environment ,
+  Value *Val2, const Environment ,
+  Environment::ValueModel ) {
+  // Join distinct boolean values preserving information about the constraints
+  // in the respective path conditions.
+  if (auto *Expr1 = dyn_cast(Val1)) {
+for (BoolValue *Constraint : Env1.getFlowConditionConstraints()) {
+  Expr1 = (*Expr1, *Constraint);
+}
+auto *Expr2 = cast(Val2);
+for (BoolValue *Constraint : Env2.getFlowConditionConstraints()) {
+  Expr2 = (*Expr2, *Constraint);
+}
+return (*Expr1, *Expr2);
+  }
+
+  // FIXME: Consider destroying `MergedValue` immediately if `ValueModel::merge`
+  // returns false to avoid storing unneeded values in `DACtx`.
+  if (Value *MergedVal = Env1.createValue(Type))
+if (Model.merge(Type, *Val1, Env1, *Val2, Env2, *MergedVal, Env1))
+  return MergedVal;
+
+  return nullptr;
+}
+
 /// Initializes a global storage value.
 static void initGlobalVar(const VarDecl , Environment ) {
   if (!D.hasGlobalStorage() ||
@@ -273,13 +302,9 @@
   continue;
 }
 
-// FIXME: Consider destroying `MergedValue` immediately if
-// `ValueModel::merge` returns false to avoid storing unneeded values in
-// `DACtx`.
-if (Value *MergedVal = createValue(Loc->getType()))
-  if (Model.merge(Loc->getType(), *Val, *this, *It->second, Other,
-  *MergedVal, *this))
-LocToVal.insert({Loc, MergedVal});
+if (Value 

[PATCH] D122766: [clang] Use forward slash as the path separator for Windows in __FILE__, __builtin_FILE(), and std::source_location

[Alan Zhao via Phabricator via cfe-commits]

ayzhao added inline comments.



Comment at: clang/lib/AST/Expr.cpp:2193
 SmallString<256> Path(PLoc.getFilename());
 Ctx.getLangOpts().remapPathPrefix(Path);
+if (Ctx.getTargetInfo().getTriple().isOSWindows()) {

hans wrote:
> I was going to say perhaps we should put a comment about why Windows needs 
> different treatment, but as we'd need to put the comment in three different 
> places, maybe this (remapPathPrefix() and make_preferred()) should be 
> factored out to a utility function?
Extracting these lines into a separate function (say, for example, 
`handle_file_macro(...)`?) makes sense. However, I'm not sure which file this 
function would belong:

* The function would need to accept a `LangOpts` and a `TargetInfo` object. 
Both of these classes are Clang specific.
* It shouldn't belong in `llvm::sys::path::Style` because the file declaring 
that namespace exists in the llvm directory, which shouldn't depend on things 
in Clang.
* We could put it in `LangOpts`, but `TargetInfo.h` includes `LangOpts.h`, so 
this would create a circular dependency.
* `TargetInfo` doesn't make much sense as it doesn't have any code generation / 
language specific logic.

WDYT?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122766

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

[PATCH] D122822: [Clang][CodeGen]Add constant array support for __builtin_dump_sturct

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

In D122822#3420057 , @yihanaa wrote:

> There is another question, which way should I print for an empty array?
>
> #1: Same as non empty array
>
>   int[0] a = [
>   ]
>
> #2:
>
>   int[0] a = []
>
> The #1's format is more uniform, and the #2 maybe looks a little clearer. 
> What do you think? @erichkeane

I'm not motivated one way or another, I can see the logic to each. I would do 
whichever doesn't require 'special' handling in any way.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122822

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

[PATCH] D122822: [Clang][CodeGen]Add constant array support for __builtin_dump_sturct

[Wang Yihan via Phabricator via cfe-commits]

yihanaa added a comment.

There is another question, which way should I print for an empty array?

#1: Same as non empty array

  int[0] a = [
  ]

#2:

  int[0] a = []

The #1's format is more uniform, and the #2 maybe looks a little clearer. What 
do you think? @erichkeane


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122822

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

[clang] 0e89090 - Use functions with prototypes when appropriate; NFC

[Aaron Ballman via cfe-commits]

Author: Aaron Ballman
Date: 2022-03-31T13:45:39-04:00
New Revision: 0e890904ea342aba088ed2a55b537ffdb0562234

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

LOG: Use functions with prototypes when appropriate; NFC

A significant number of our tests in C accidentally use functions
without prototypes. This patch converts the function signatures to have
a prototype for the situations where the test is not specific to K C
declarations. e.g.,

  void func();

becomes

  void func(void);

Added: 


Modified: 
clang/test/Parser/opencl-atomics-cl20.cl
clang/test/Parser/vector-cast-define.cl
clang/test/Preprocessor/macro_variadic.cl
clang/test/Sema/builtins.cl
clang/test/SemaOpenCL/address-spaces-conversions-cl2.0.cl
clang/test/SemaOpenCL/clang-builtin-version.cl
clang/test/SemaOpenCL/clk_event_t.cl
clang/test/SemaOpenCL/fdeclare-opencl-builtins.cl
clang/test/SemaOpenCL/invalid-assignment-constant-address-space.cl
clang/test/SemaOpenCL/invalid-pipe-builtin-cl2.0.cl
clang/test/SemaOpenCL/invalid-pipes-cl1.2.cl
clang/test/SemaOpenCL/sampler_t.cl
clang/test/SemaOpenCL/shifts.cl
clang/test/SemaOpenCL/storageclass-cl20.cl
clang/test/SemaOpenCL/storageclass.cl
clang/test/SemaOpenCL/vec_compare.cl
clang/test/SemaOpenCL/vector_inc_dec_ops.cl
clang/test/SemaOpenCL/vector_swizzle_length.cl

Removed: 




diff  --git a/clang/test/Parser/opencl-atomics-cl20.cl 
b/clang/test/Parser/opencl-atomics-cl20.cl
index 50866dd61591e..2648142f28e7c 100644
--- a/clang/test/Parser/opencl-atomics-cl20.cl
+++ b/clang/test/Parser/opencl-atomics-cl20.cl
@@ -8,7 +8,7 @@
 #define LANG_VER_OK
 #endif
 
-void atomic_types_test() {
+void atomic_types_test(void) {
 // OpenCL v2.0 s6.13.11.6 defines supported atomic types.
 
 // Non-optional types

diff  --git a/clang/test/Parser/vector-cast-define.cl 
b/clang/test/Parser/vector-cast-define.cl
index ec4eba7a78488..44acf9a6b0e62 100644
--- a/clang/test/Parser/vector-cast-define.cl
+++ b/clang/test/Parser/vector-cast-define.cl
@@ -3,7 +3,7 @@
 
 typedef int int3 __attribute__((ext_vector_type(3)));
 
-void test()
+void test(void)
 {
 int index = (int3)(1, 2, 3).x * (int3)(3, 2, 1).y;
 }

diff  --git a/clang/test/Preprocessor/macro_variadic.cl 
b/clang/test/Preprocessor/macro_variadic.cl
index e588964871216..04e96e3c21465 100644
--- a/clang/test/Preprocessor/macro_variadic.cl
+++ b/clang/test/Preprocessor/macro_variadic.cl
@@ -15,7 +15,7 @@
 
 int printf(__constant const char *st, ...);
 
-void foo() {
+void foo(void) {
   NO_VAR_FUNC(1, 2, 3);
   VAR_FUNC(1, 2, 3);
 #if !__OPENCL_CPP_VERSION__

diff  --git a/clang/test/Sema/builtins.cl b/clang/test/Sema/builtins.cl
index 7cde5e1a9e4d4..c6bdbfc827b84 100644
--- a/clang/test/Sema/builtins.cl
+++ b/clang/test/Sema/builtins.cl
@@ -6,6 +6,6 @@ kernel void test(global float *out, global float *in, global 
int* in2) {
   out[0] = __builtin_frexpf(in[0], in2);
 }
 
-void pr28651() {
+void pr28651(void) {
   __builtin_alloca(value); // expected-error{{use of undeclared identifier}}
 }

diff  --git a/clang/test/SemaOpenCL/address-spaces-conversions-cl2.0.cl 
b/clang/test/SemaOpenCL/address-spaces-conversions-cl2.0.cl
index 482254190789a..5942efb624ad3 100644
--- a/clang/test/SemaOpenCL/address-spaces-conversions-cl2.0.cl
+++ b/clang/test/SemaOpenCL/address-spaces-conversions-cl2.0.cl
@@ -385,7 +385,7 @@ void test_conversion(__global int *arg_glob, __local int 
*arg_loc,
 #endif
 }
 
-void test_ternary() {
+void test_ternary(void) {
   AS int *var_cond;
   __generic int *var_gen;
   __global int *var_glob;
@@ -500,7 +500,7 @@ void test_ternary() {
 #endif
 }
 
-void test_pointer_chains() {
+void test_pointer_chains(void) {
   AS int *AS *var_as_as_int;
   AS int *AS_COMP *var_asc_as_int;
   AS_INCOMP int *AS_COMP *var_asc_asn_int;

diff  --git a/clang/test/SemaOpenCL/clang-builtin-version.cl 
b/clang/test/SemaOpenCL/clang-builtin-version.cl
index 7111db1784c60..a3e5b5bd80fff 100644
--- a/clang/test/SemaOpenCL/clang-builtin-version.cl
+++ b/clang/test/SemaOpenCL/clang-builtin-version.cl
@@ -4,7 +4,7 @@
 
 // Confirm CL2.0 Clang builtins are not available in earlier versions and in 
OpenCL C 3.0 without required features.
 
-kernel void dse_builtins() {
+kernel void dse_builtins(void) {
   int tmp;
   enqueue_kernel(tmp, tmp, tmp, ^(void) { // expected-error{{implicit 
declaration of function 'enqueue_kernel' is invalid in OpenCL}}
 return;
@@ -22,7 +22,7 @@ kernel void dse_builtins() {
 #endif
 }
 
-void pipe_builtins() {
+void pipe_builtins(void) {
   int tmp;
 
   foo(void); // expected-error{{implicit declaration of function 'foo' is 
invalid in OpenCL}}

diff  --git a/clang/test/SemaOpenCL/clk_event_t.cl 
b/clang/test/SemaOpenCL/clk_event_t.cl
index 

[PATCH] D115907: [misexpect] Re-implement MisExpect Diagnostics

[Paul Kirth via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG46774df30715: [misexpect] Re-implement MisExpect Diagnostics 
(authored by paulkirth).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115907

Files:
  clang/docs/MisExpect.rst
  clang/docs/ReleaseNotes.rst
  clang/docs/index.rst
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Basic/CodeGenOptions.h
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/include/clang/Basic/DiagnosticFrontendKinds.td
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/CodeGen/CodeGenAction.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/Profile/Inputs/misexpect-branch-nonconst-expect-arg.proftext
  clang/test/Profile/Inputs/misexpect-branch.proftext
  clang/test/Profile/Inputs/misexpect-switch-default-only.proftext
  clang/test/Profile/Inputs/misexpect-switch-default.proftext
  clang/test/Profile/Inputs/misexpect-switch-nonconst.proftext
  clang/test/Profile/Inputs/misexpect-switch.proftext
  clang/test/Profile/misexpect-branch-cold.c
  clang/test/Profile/misexpect-branch-nonconst-expected-val.c
  clang/test/Profile/misexpect-branch-unpredictable.c
  clang/test/Profile/misexpect-branch.c
  clang/test/Profile/misexpect-switch-default.c
  clang/test/Profile/misexpect-switch-nonconst.c
  clang/test/Profile/misexpect-switch-only-default-case.c
  clang/test/Profile/misexpect-switch.c
  llvm/docs/MisExpect.rst
  llvm/docs/UserGuides.rst
  llvm/include/llvm/IR/DiagnosticInfo.h
  llvm/include/llvm/IR/LLVMContext.h
  llvm/include/llvm/Target/TargetOptions.h
  llvm/include/llvm/Transforms/Utils/MisExpect.h
  llvm/lib/IR/DiagnosticInfo.cpp
  llvm/lib/IR/LLVMContext.cpp
  llvm/lib/IR/LLVMContextImpl.h
  llvm/lib/Transforms/IPO/SampleProfile.cpp
  llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp
  llvm/lib/Transforms/Scalar/LowerExpectIntrinsic.cpp
  llvm/lib/Transforms/Utils/CMakeLists.txt
  llvm/lib/Transforms/Utils/MisExpect.cpp
  llvm/test/Transforms/PGOProfile/Inputs/misexpect-branch-correct.proftext
  llvm/test/Transforms/PGOProfile/Inputs/misexpect-branch-threshold.proftext
  llvm/test/Transforms/PGOProfile/Inputs/misexpect-branch.proftext
  llvm/test/Transforms/PGOProfile/Inputs/misexpect-switch-correct.proftext
  llvm/test/Transforms/PGOProfile/Inputs/misexpect-switch.proftext
  llvm/test/Transforms/PGOProfile/misexpect-branch-correct.ll
  llvm/test/Transforms/PGOProfile/misexpect-branch-stripped.ll
  llvm/test/Transforms/PGOProfile/misexpect-branch-unpredictable.ll
  llvm/test/Transforms/PGOProfile/misexpect-branch.ll
  llvm/test/Transforms/PGOProfile/misexpect-switch-default.ll
  llvm/test/Transforms/PGOProfile/misexpect-switch.ll

Index: llvm/test/Transforms/PGOProfile/misexpect-switch.ll
===
--- /dev/null
+++ llvm/test/Transforms/PGOProfile/misexpect-switch.ll
@@ -0,0 +1,285 @@
+; Test misexpect diagnostics handle swich statements, and report source locations correctly
+
+; RUN: llvm-profdata merge %S/Inputs/misexpect-switch.proftext -o %t.profdata
+; RUN: llvm-profdata merge %S/Inputs/misexpect-switch-correct.proftext -o %t.c.profdata
+
+; RUN: opt < %s -passes="function(lower-expect),pgo-instr-use" -pgo-test-profile-file=%t.profdata -pgo-warn-misexpect -S 2>&1 | FileCheck %s --check-prefix=WARNING
+; RUN: opt < %s -passes="function(lower-expect),pgo-instr-use" -pgo-test-profile-file=%t.profdata -pass-remarks=misexpect -S 2>&1 | FileCheck %s --check-prefix=REMARK
+; RUN: opt < %s -passes="function(lower-expect),pgo-instr-use" -pgo-test-profile-file=%t.profdata -pgo-warn-misexpect -pass-remarks=misexpect -S 2>&1 | FileCheck %s --check-prefix=BOTH
+; RUN: opt < %s -passes="function(lower-expect),pgo-instr-use" -pgo-test-profile-file=%t.profdata -S 2>&1 | FileCheck %s --check-prefix=DISABLED
+
+; RUN: opt < %s -passes="function(lower-expect),pgo-instr-use" -pgo-test-profile-file=%t.c.profdata -pgo-warn-misexpect -pass-remarks=misexpect -S 2>&1 | FileCheck %s --check-prefix=CORRECT
+
+; WARNING-DAG: warning: misexpect-switch.c:26:30: 0.00%
+; WARNING-NOT: remark: misexpect-switch.c:26:30: Potential performance regression from use of the llvm.expect intrinsic: Annotation was correct on 0.00% (0 / 8112) of profiled executions.
+
+; REMARK-NOT: warning: misexpect-switch.c:26:30: 0.00%
+; REMARK-DAG: remark: misexpect-switch.c:26:30: Potential performance regression from use of the llvm.expect intrinsic: Annotation was correct on 0.00% (0 / 8112) of profiled executions.
+
+; BOTH-DAG: warning: misexpect-switch.c:26:30: 0.00%
+; BOTH-DAG: remark: misexpect-switch.c:26:30: Potential performance regression from use of the llvm.expect intrinsic: Annotation was correct on 0.00% (0 / 8112) of profiled 

[PATCH] D122822: [Clang][CodeGen]Add constant array support for __builtin_dump_sturct

[Erich Keane via Phabricator via cfe-commits]

erichkeane added inline comments.



Comment at: clang/lib/CodeGen/CGBuiltin.cpp:2071
+  if (Types.find(Type) == Types.end())
+return Types[Context.VoidPtrTy];
+  return Types[Type];

yihanaa wrote:
> erichkeane wrote:
> > yihanaa wrote:
> > > erichkeane wrote:
> > > > When can we hit this case?  Does this mean that any types we dont 
> > > > understand we are just treating as void ptrs?  
> > > The previous version treated it as a void *ptr for unsupported types, 
> > > such as arrays (before this patch) and __int128. This is the case i saw 
> > > in an issue. I also think that for unsupported types, we should generate 
> > > a clearer message saying "This type is temporarily not supported", do you 
> > > have any good ideas?
> > A diagnostic about ''TYPE' not yet supported by __bultin...' probably isn't 
> > a bad idea if we have the ability.
> I think maybe we can emit an error(or warning? in this way, it may be 
> necessary to handle those unsupported types) diag msg, but that might be 
> worth a separate commit. What do you think?
As long as we are good about 'notes' to tell the user how we GOT to there, I'd 
think the warning would be useful.  I'd probably just do something like, 
"FunnyType Foo = " rather than do something about it.

I'd be OK with that being a separate commit.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122822

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

[PATCH] D122822: [Clang][CodeGen]Add constant array support for __builtin_dump_sturct

[Wang Yihan via Phabricator via cfe-commits]

yihanaa added inline comments.



Comment at: clang/lib/CodeGen/CGBuiltin.cpp:2071
+  if (Types.find(Type) == Types.end())
+return Types[Context.VoidPtrTy];
+  return Types[Type];

erichkeane wrote:
> yihanaa wrote:
> > erichkeane wrote:
> > > When can we hit this case?  Does this mean that any types we dont 
> > > understand we are just treating as void ptrs?  
> > The previous version treated it as a void *ptr for unsupported types, such 
> > as arrays (before this patch) and __int128. This is the case i saw in an 
> > issue. I also think that for unsupported types, we should generate a 
> > clearer message saying "This type is temporarily not supported", do you 
> > have any good ideas?
> A diagnostic about ''TYPE' not yet supported by __bultin...' probably isn't a 
> bad idea if we have the ability.
I think maybe we can emit an error(or warning? in this way, it may be necessary 
to handle those unsupported types) diag msg, but that might be worth a separate 
commit. What do you think?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122822

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

[PATCH] D122781: This patch aims to conform AMDGPUOpenMP driver sanitizer changes w.r.t HIPAMD toolchain.

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added a comment.

This patch can be retitled as "Refactor sanitizer options handling for AMDGPU 
Toolchain".


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122781

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

[PATCH] D122471: [IR] Require intrinsic struct return type to be anonymous

[Mikhail Goncharov via Phabricator via cfe-commits]

goncharov added a comment.

Hi, I have found this test to cs-preinline-sample-profile.test to be flaky 
around 10% of the runs. Could you please check what might be the issue?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122471

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

[PATCH] D122822: [Clang][CodeGen]Add constant array support for __builtin_dump_sturct

[Erich Keane via Phabricator via cfe-commits]

erichkeane added inline comments.



Comment at: clang/lib/CodeGen/CGBuiltin.cpp:2048
+  QualType Type) {
+  static llvm::DenseMap Types;
   if (Types.empty()) {

yihanaa wrote:
> erichkeane wrote:
> > Instead of this initialization, can we make this a constexpr 
> > constructor/collection of some sort and instantiate it inline?  
> > Instead of this initialization, can we make this a constexpr 
> > constructor/collection of some sort and instantiate it inline?  
> 
> I don't have a good idea because it relies on Context to get some types like 
> const char *
We could at minimum do a in inline-initializer instead of the branch below.



Comment at: clang/lib/CodeGen/CGBuiltin.cpp:2071
+  if (Types.find(Type) == Types.end())
+return Types[Context.VoidPtrTy];
+  return Types[Type];

yihanaa wrote:
> erichkeane wrote:
> > When can we hit this case?  Does this mean that any types we dont 
> > understand we are just treating as void ptrs?  
> The previous version treated it as a void *ptr for unsupported types, such as 
> arrays (before this patch) and __int128. This is the case i saw in an issue. 
> I also think that for unsupported types, we should generate a clearer message 
> saying "This type is temporarily not supported", do you have any good ideas?
A diagnostic about ''TYPE' not yet supported by __bultin...' probably isn't a 
bad idea if we have the ability.



Comment at: clang/lib/CodeGen/CGBuiltin.cpp:2257
+
+if (CanonicalType->isConstantArrayType()) {
+  GString = CGF.Builder.CreateGlobalStringPtr(

yihanaa wrote:
> erichkeane wrote:
> > Do we want to do anything for the OTHER array types?  
> I understand struct fields must have a constant size, and I see" 'variable 
> length array in structure' extension will never be supported" from clang's 
> diagnostics
Ah, right, good point.  I think this + the diagnostic discussed above would 
make that alright.

There is ALSO the 'matrix' type that we could emit, but that might be worth a 
separate commit.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122822

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

[PATCH] D122830: [clang][dataflow] Add support for (built-in) (in)equality operators

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

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



Comment at: clang/lib/Analysis/FlowSensitive/Transfer.cpp:56
+Env.getValue(*RHSNorm, SkipPast::Reference)))
+  return Env.makeIff(*LHSValue, *RHSValue);
+

xazax.hun wrote:
> I think a possible alternative way to handle this is to do some sort of 
> "unification" for the two values. The current approach is superior in the 
> sense that solving and generating constraints are clearly separate. Also this 
> might be better for generating useful error messages. In case the solver ever 
> becomes a bottleneck, I wonder whether replacing this with a unification step 
> would speed things up a bit. 
> Although that would only work for the equality case.
That's a good point. Although I wonder if we could get the best of both worlds 
by adding a unification pre-step to the solver itself, basically looking for 
`iff` patterns of this sort. Then, the solver could keep a record of such 
unifications which it could export for diagnostic purposes.

On a related note, another potential use of unification is in equivalence 
testing of environments. Instead of strict equality of flow conditions, for 
example, I could imagine equivalence up to unification (aka. renaming of atoms).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122830

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

[PATCH] D122069: [Object] Add binary format for bundling offloading metadata

[Joseph Huber via Phabricator via cfe-commits]

jhuber6 added a comment.

Ping, I'd like to finalize the new driver in time for the GPU newsletter and 
the LLVM Performance Workshop at CGO.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122069

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

[PATCH] D122273: [clang][dataflow] Fix handling of base-class fields

[Gábor Horváth via Phabricator via cfe-commits]

xazax.hun accepted this revision.
xazax.hun added inline comments.



Comment at: clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp:185
+  // base classes, because they are not visible in derived classes.
+  getFieldsFromClassHierarchy(Base.getType(), /*IgnorePrivateFields=*/true,
+  Fields);

ymandel wrote:
> xazax.hun wrote:
> > Will this work well for all cases of diamond shape inheritance?
> > 
> > E.g.:
> > ```
> > struct A { int a; };
> > struct B : virtual A { int b; };
> > struct C : virtual A { int c; };
> > struct D : B, C { int d; };
> > ```
> > 
> > In the above code, I would expect the field `a` to appear only once. I 
> > guess this should work well because of the set representation although we 
> > will visit A twice.
> > 
> > On the other hand, consider:
> > ```
> > struct A { int a; };
> > struct B : A { int b; };
> > struct C : A { int c; };
> > struct D : B, C { int d; };
> > ```
> > 
> > Here, in fact, we have 2 instances of the field `a`. Both `B::a` and `C::a` 
> > are part of `D`. I suspect that the current implementation might not handle 
> > this.
> Good point, no. But, I'm not sure that fixing this here would be enough - I 
> think we'd also need to revisit how we model structs, since `getChild` takes 
> a decl as argument -- it doesn't support multiple versions of the same field 
> decl.
> 
> I added a FIXME since this seems a larger issue, but let me know if you think 
> it needs fixing now.
I think the argument of `getChild` needs fixing as well is strong enough to 
push this to a separate PR.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122273

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

[PATCH] D122627: [HLSL] Fix MSFT Attribute parsing, add numthreads

[Chris Bieneman via Phabricator via cfe-commits]

beanz added a comment.

I just pushed one more commit rG19054163e11a6632b4973c936e5aa93ec742c866 
 that 
further refines the subject specification and covers the merging attributes and 
conflicting attributes on the same decl.

Thank you so much for the spectacular review feedback!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122627

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

[clang] 1905416 - [HLSL] Further improve to numthreads diagnostics

[Chris Bieneman via cfe-commits]

Author: Chris Bieneman
Date: 2022-03-31T11:34:01-05:00
New Revision: 19054163e11a6632b4973c936e5aa93ec742c866

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

LOG: [HLSL] Further improve to numthreads diagnostics

This adds diagnostics for conflicting attributes on the same
declarataion, conflicting attributes on a forward and final
declaration, and defines a more narrowly scoped HLSLEntry attribute
target.

Big shout out to @aaron.ballman for the great feedback and review on
this!

Added: 


Modified: 
clang/include/clang/Basic/Attr.td
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/include/clang/Sema/Sema.h
clang/lib/Sema/SemaDecl.cpp
clang/lib/Sema/SemaDeclAttr.cpp
clang/test/SemaHLSL/num_threads.hlsl

Removed: 




diff  --git a/clang/include/clang/Basic/Attr.td 
b/clang/include/clang/Basic/Attr.td
index 97b1027742f60..4789493399ec2 100644
--- a/clang/include/clang/Basic/Attr.td
+++ b/clang/include/clang/Basic/Attr.td
@@ -126,8 +126,10 @@ def FunctionTmpl
  FunctionDecl::TK_FunctionTemplate}],
 "function templates">;
 
-def GlobalFunction
-: SubsetSubjectisGlobal()}], "global functions">;
+def HLSLEntry
+: SubsetSubjectisExternallyVisible() && !isa(S)}],
+"global functions">;
 
 def ClassTmpl : SubsetSubjectgetDescribedClassTemplate()}],
   "class templates">;
@@ -3946,7 +3948,7 @@ def Error : InheritableAttr {
 def HLSLNumThreads: InheritableAttr {
   let Spellings = [Microsoft<"numthreads">];
   let Args = [IntArgument<"X">, IntArgument<"Y">, IntArgument<"Z">];
-  let Subjects = SubjectList<[GlobalFunction]>;
+  let Subjects = SubjectList<[HLSLEntry]>;
   let LangOpts = [HLSL];
   let Documentation = [NumThreadsDocs];
 }

diff  --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td 
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index a272cb741270f..aec172c39ed9a 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -11570,6 +11570,7 @@ def err_hlsl_attr_unsupported_in_stage : 
Error<"attribute %0 is unsupported in %
 
 def err_hlsl_numthreads_argument_oor : Error<"argument '%select{X|Y|Z}0' to 
numthreads attribute cannot exceed %1">;
 def err_hlsl_numthreads_invalid : Error<"total number of threads cannot exceed 
%0">;
+def err_hlsl_attribute_param_mismatch : Error<"%0 attribute parameters do not 
match the previous declaration">;
 
 } // end of sema component.
 

diff  --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 6523c3001c294..c0ad55d52bb31 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -3471,6 +3471,9 @@ class Sema final {
   EnforceTCBLeafAttr *mergeEnforceTCBLeafAttr(Decl *D,
   const EnforceTCBLeafAttr );
   BTFDeclTagAttr *mergeBTFDeclTagAttr(Decl *D, const BTFDeclTagAttr );
+  HLSLNumThreadsAttr *mergeHLSLNumThreadsAttr(Decl *D,
+  const AttributeCommonInfo ,
+  int X, int Y, int Z);
 
   void mergeDeclAttributes(NamedDecl *New, Decl *Old,
AvailabilityMergeKind AMK = AMK_Redeclaration);

diff  --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index b913f805bc877..1e25346fde6f6 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -2770,6 +2770,9 @@ static bool mergeDeclAttribute(Sema , NamedDecl *D,
 NewAttr = S.mergeEnforceTCBLeafAttr(D, *TCBLA);
   else if (const auto *BTFA = dyn_cast(Attr))
 NewAttr = S.mergeBTFDeclTagAttr(D, *BTFA);
+  else if (const auto *NT = dyn_cast(Attr))
+NewAttr =
+S.mergeHLSLNumThreadsAttr(D, *NT, NT->getX(), NT->getY(), NT->getZ());
   else if (Attr->shouldInheritEvenIfAlreadyPresent() || !DeclHasAttr(D, Attr))
 NewAttr = cast(Attr->clone(S.Context));
 

diff  --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp
index 87e16635f3021..4b5201db7517c 100644
--- a/clang/lib/Sema/SemaDeclAttr.cpp
+++ b/clang/lib/Sema/SemaDeclAttr.cpp
@@ -6892,7 +6892,22 @@ static void handleHLSLNumThreadsAttr(Sema , Decl *D, 
const ParsedAttr ) {
 return;
   }
 
-  D->addAttr(::new (S.Context) HLSLNumThreadsAttr(S.Context, AL, X, Y, Z));
+  HLSLNumThreadsAttr *NewAttr = S.mergeHLSLNumThreadsAttr(D, AL, X, Y, Z);
+  if (NewAttr)
+D->addAttr(NewAttr);
+}
+
+HLSLNumThreadsAttr *Sema::mergeHLSLNumThreadsAttr(Decl *D,
+  const AttributeCommonInfo 
,
+  int X, int Y, int Z) {
+  if (HLSLNumThreadsAttr *NT = D->getAttr()) {
+if 

[PATCH] D122273: [clang][dataflow] Fix handling of base-class fields

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel added a comment.

Thanks for the reviews.




Comment at: clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp:185
+  // base classes, because they are not visible in derived classes.
+  getFieldsFromClassHierarchy(Base.getType(), /*IgnorePrivateFields=*/true,
+  Fields);

xazax.hun wrote:
> Will this work well for all cases of diamond shape inheritance?
> 
> E.g.:
> ```
> struct A { int a; };
> struct B : virtual A { int b; };
> struct C : virtual A { int c; };
> struct D : B, C { int d; };
> ```
> 
> In the above code, I would expect the field `a` to appear only once. I guess 
> this should work well because of the set representation although we will 
> visit A twice.
> 
> On the other hand, consider:
> ```
> struct A { int a; };
> struct B : A { int b; };
> struct C : A { int c; };
> struct D : B, C { int d; };
> ```
> 
> Here, in fact, we have 2 instances of the field `a`. Both `B::a` and `C::a` 
> are part of `D`. I suspect that the current implementation might not handle 
> this.
Good point, no. But, I'm not sure that fixing this here would be enough - I 
think we'd also need to revisit how we model structs, since `getChild` takes a 
decl as argument -- it doesn't support multiple versions of the same field decl.

I added a FIXME since this seems a larger issue, but let me know if you think 
it needs fixing now.



Comment at: clang/unittests/Analysis/FlowSensitive/TransferTest.cpp:1014
+  std::string Code = R"(
+struct A {
+  int Bar;

sgatev wrote:
> Add a similar test with `class` instead of `struct`?
I went with a simpler test for struct, only verifying that the default is 
understood correctly (and differently than class), but happy to expand it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122273

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

[PATCH] D122273: [clang][dataflow] Fix handling of base-class fields

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel updated this revision to Diff 419499.
ymandel marked 5 inline comments as done.
ymandel added a comment.

adjusted test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122273

Files:
  clang/include/clang/Analysis/FlowSensitive/StorageLocation.h
  clang/include/clang/Analysis/FlowSensitive/Value.h
  clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
  clang/unittests/Analysis/FlowSensitive/TransferTest.cpp

Index: clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
@@ -1009,6 +1009,157 @@
   });
 }
 
+TEST_F(TransferTest, DerivedBaseMemberClass) {
+  std::string Code = R"(
+class A {
+  int ADefault;
+protected:
+  int AProtected;
+private:
+  int APrivate;
+public:
+  int APublic;
+};
+
+class B : public A {
+  int BDefault;
+protected:
+  int BProtected;
+private:
+  int BPrivate;
+};
+
+void target() {
+  B Foo;
+  // [[p]]
+}
+  )";
+  runDataflow(
+  Code, [](llvm::ArrayRef<
+   std::pair>>
+   Results,
+   ASTContext ) {
+ASSERT_THAT(Results, ElementsAre(Pair("p", _)));
+const Environment  = Results[0].second.Env;
+
+const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
+ASSERT_THAT(FooDecl, NotNull());
+ASSERT_TRUE(FooDecl->getType()->isRecordType());
+
+// Derived-class fields.
+const FieldDecl *BDefaultDecl = nullptr;
+const FieldDecl *BProtectedDecl = nullptr;
+const FieldDecl *BPrivateDecl = nullptr;
+for (const FieldDecl *Field :
+ FooDecl->getType()->getAsRecordDecl()->fields()) {
+  if (Field->getNameAsString() == "BDefault") {
+BDefaultDecl = Field;
+  } else if (Field->getNameAsString() == "BProtected") {
+BProtectedDecl = Field;
+  } else if (Field->getNameAsString() == "BPrivate") {
+BPrivateDecl = Field;
+  } else {
+FAIL() << "Unexpected field: " << Field->getNameAsString();
+  }
+}
+ASSERT_THAT(BDefaultDecl, NotNull());
+ASSERT_THAT(BProtectedDecl, NotNull());
+ASSERT_THAT(BPrivateDecl, NotNull());
+
+// Base-class fields.
+const FieldDecl *ADefaultDecl = nullptr;
+const FieldDecl *APrivateDecl = nullptr;
+const FieldDecl *AProtectedDecl = nullptr;
+const FieldDecl *APublicDecl = nullptr;
+for (const clang::CXXBaseSpecifier  :
+ FooDecl->getType()->getAsCXXRecordDecl()->bases()) {
+  QualType BaseType = Base.getType();
+  ASSERT_TRUE(BaseType->isRecordType());
+  for (const FieldDecl *Field : BaseType->getAsRecordDecl()->fields()) {
+if (Field->getNameAsString() == "ADefault") {
+  ADefaultDecl = Field;
+} else if (Field->getNameAsString() == "AProtected") {
+  AProtectedDecl = Field;
+} else if (Field->getNameAsString() == "APrivate") {
+  APrivateDecl = Field;
+} else if (Field->getNameAsString() == "APublic") {
+  APublicDecl = Field;
+} else {
+  FAIL() << "Unexpected field: " << Field->getNameAsString();
+}
+  }
+}
+ASSERT_THAT(ADefaultDecl, NotNull());
+ASSERT_THAT(AProtectedDecl, NotNull());
+ASSERT_THAT(APrivateDecl, NotNull());
+ASSERT_THAT(APublicDecl, NotNull());
+
+const auto *FooLoc = cast(
+Env.getStorageLocation(*FooDecl, SkipPast::None));
+const auto  = *cast(Env.getValue(*FooLoc));
+EXPECT_THAT(FooVal.getChild(*APublicDecl), NotNull());
+
+// Base-class fields.
+EXPECT_THAT(FooVal.getChild(*ADefaultDecl), IsNull());
+EXPECT_THAT(FooVal.getChild(*APrivateDecl), IsNull());
+EXPECT_THAT(FooVal.getChild(*AProtectedDecl), NotNull());
+EXPECT_THAT(FooVal.getChild(*APublicDecl), NotNull());
+
+// Derived-class fields.
+EXPECT_THAT(FooVal.getChild(*BDefaultDecl), NotNull());
+EXPECT_THAT(FooVal.getChild(*BProtectedDecl), NotNull());
+EXPECT_THAT(FooVal.getChild(*BPrivateDecl), NotNull());
+  });
+}
+
+TEST_F(TransferTest, DerivedBaseMemberStructDefault) {
+  std::string Code = R"(
+struct A {
+  int Bar;
+};
+struct B : public A {
+};
+
+void target() {
+  B Foo;
+  // [[p]]
+}
+  )";
+  runDataflow(
+  Code, [](llvm::ArrayRef<
+   std::pair>>
+   Results,
+   ASTContext ) {
+ASSERT_THAT(Results, ElementsAre(Pair("p", _)));
+const Environment  = Results[0].second.Env;
+
+

[PATCH] D115620: [AArch64] Lowering and legalization of strict FP16

[John Brawn via Phabricator via cfe-commits]

john.brawn updated this revision to Diff 419494.
john.brawn added a comment.

Adjusted formatting slightly.


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

https://reviews.llvm.org/D115620

Files:
  clang/test/CodeGen/aarch64-v8.2a-fp16-intrinsics-constrained.c
  llvm/lib/CodeGen/SelectionDAG/LegalizeDAG.cpp
  llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
  llvm/test/CodeGen/AArch64/fp-intrinsics-fp16.ll
  llvm/test/CodeGen/AArch64/fp-intrinsics.ll

Index: llvm/test/CodeGen/AArch64/fp-intrinsics.ll
===
--- llvm/test/CodeGen/AArch64/fp-intrinsics.ll
+++ llvm/test/CodeGen/AArch64/fp-intrinsics.ll
@@ -1,5 +1,5 @@
-; RUN: llc -mtriple=aarch64-none-eabi %s -disable-strictnode-mutation -o - | FileCheck %s
-; RUN: llc -mtriple=aarch64-none-eabi -global-isel=true -global-isel-abort=2 -disable-strictnode-mutation %s -o - | FileCheck %s
+; RUN: llc -mtriple=aarch64-none-eabi %s -o - | FileCheck %s
+; RUN: llc -mtriple=aarch64-none-eabi -global-isel=true -global-isel-abort=2 %s -o - | FileCheck %s
 
 ; Check that constrained fp intrinsics are correctly lowered.
 
Index: llvm/test/CodeGen/AArch64/fp-intrinsics-fp16.ll
===
--- /dev/null
+++ llvm/test/CodeGen/AArch64/fp-intrinsics-fp16.ll
@@ -0,0 +1,1173 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc -mtriple=aarch64-none-eabi %s -o - | FileCheck %s --check-prefixes=CHECK,CHECK-NOFP16
+; RUN: llc -mtriple=aarch64-none-eabi -mattr=+fullfp16 %s -o - | FileCheck %s --check-prefixes=CHECK,CHECK-FP16
+; RUN: llc -mtriple=aarch64-none-eabi -global-isel=true -global-isel-abort=2 %s -o - | FileCheck %s --check-prefixes=CHECK,CHECK-NOFP16
+; RUN: llc -mtriple=aarch64-none-eabi -global-isel=true -global-isel-abort=2 -mattr=+fullfp16 %s -o - | FileCheck %s --check-prefixes=CHECK,CHECK-FP16
+
+; Check that constrained fp intrinsics are correctly lowered.
+
+
+; Half-precision intrinsics
+
+define half @add_f16(half %x, half %y) #0 {
+; CHECK-NOFP16-LABEL: add_f16:
+; CHECK-NOFP16:   // %bb.0:
+; CHECK-NOFP16-NEXT:fcvt s1, h1
+; CHECK-NOFP16-NEXT:fcvt s0, h0
+; CHECK-NOFP16-NEXT:fadd s0, s0, s1
+; CHECK-NOFP16-NEXT:fcvt h0, s0
+; CHECK-NOFP16-NEXT:ret
+;
+; CHECK-FP16-LABEL: add_f16:
+; CHECK-FP16:   // %bb.0:
+; CHECK-FP16-NEXT:fadd h0, h0, h1
+; CHECK-FP16-NEXT:ret
+  %val = call half @llvm.experimental.constrained.fadd.f16(half %x, half %y, metadata !"round.tonearest", metadata !"fpexcept.strict") #0
+  ret half %val
+}
+
+define half @sub_f16(half %x, half %y) #0 {
+; CHECK-NOFP16-LABEL: sub_f16:
+; CHECK-NOFP16:   // %bb.0:
+; CHECK-NOFP16-NEXT:fcvt s1, h1
+; CHECK-NOFP16-NEXT:fcvt s0, h0
+; CHECK-NOFP16-NEXT:fsub s0, s0, s1
+; CHECK-NOFP16-NEXT:fcvt h0, s0
+; CHECK-NOFP16-NEXT:ret
+;
+; CHECK-FP16-LABEL: sub_f16:
+; CHECK-FP16:   // %bb.0:
+; CHECK-FP16-NEXT:fsub h0, h0, h1
+; CHECK-FP16-NEXT:ret
+  %val = call half @llvm.experimental.constrained.fsub.f16(half %x, half %y, metadata !"round.tonearest", metadata !"fpexcept.strict") #0
+  ret half %val
+}
+
+define half @mul_f16(half %x, half %y) #0 {
+; CHECK-NOFP16-LABEL: mul_f16:
+; CHECK-NOFP16:   // %bb.0:
+; CHECK-NOFP16-NEXT:fcvt s1, h1
+; CHECK-NOFP16-NEXT:fcvt s0, h0
+; CHECK-NOFP16-NEXT:fmul s0, s0, s1
+; CHECK-NOFP16-NEXT:fcvt h0, s0
+; CHECK-NOFP16-NEXT:ret
+;
+; CHECK-FP16-LABEL: mul_f16:
+; CHECK-FP16:   // %bb.0:
+; CHECK-FP16-NEXT:fmul h0, h0, h1
+; CHECK-FP16-NEXT:ret
+  %val = call half @llvm.experimental.constrained.fmul.f16(half %x, half %y, metadata !"round.tonearest", metadata !"fpexcept.strict") #0
+  ret half %val
+}
+
+define half @div_f16(half %x, half %y) #0 {
+; CHECK-NOFP16-LABEL: div_f16:
+; CHECK-NOFP16:   // %bb.0:
+; CHECK-NOFP16-NEXT:fcvt s1, h1
+; CHECK-NOFP16-NEXT:fcvt s0, h0
+; CHECK-NOFP16-NEXT:fdiv s0, s0, s1
+; CHECK-NOFP16-NEXT:fcvt h0, s0
+; CHECK-NOFP16-NEXT:ret
+;
+; CHECK-FP16-LABEL: div_f16:
+; CHECK-FP16:   // %bb.0:
+; CHECK-FP16-NEXT:fdiv h0, h0, h1
+; CHECK-FP16-NEXT:ret
+  %val = call half @llvm.experimental.constrained.fdiv.f16(half %x, half %y, metadata !"round.tonearest", metadata !"fpexcept.strict") #0
+  ret half %val
+}
+
+define half @frem_f16(half %x, half %y) #0 {
+; CHECK-LABEL: frem_f16:
+; CHECK:   // %bb.0:
+; CHECK-NEXT:str x30, [sp, #-16]! // 8-byte Folded Spill
+; CHECK-NEXT:.cfi_def_cfa_offset 16
+; CHECK-NEXT:.cfi_offset w30, -16
+; CHECK-NEXT:fcvt s1, h1
+; CHECK-NEXT:fcvt s0, h0
+; CHECK-NEXT:bl fmodf
+; CHECK-NEXT:fcvt h0, s0
+; CHECK-NEXT:ldr x30, [sp], #16 // 8-byte Folded Reload
+; CHECK-NEXT:ret
+  %val = call half @llvm.experimental.constrained.frem.f16(half %x, half %y, metadata !"round.tonearest", metadata !"fpexcept.strict") #0
+  ret half %val
+}
+
+define half @fma_f16(half 

[PATCH] D122822: [Clang][CodeGen]Add constant array support for __builtin_dump_sturct

[Wang Yihan via Phabricator via cfe-commits]

yihanaa added a comment.

Thanks for your suggestion @erichkeane !




Comment at: clang/lib/CodeGen/CGBuiltin.cpp:2048
+  QualType Type) {
+  static llvm::DenseMap Types;
   if (Types.empty()) {

erichkeane wrote:
> Instead of this initialization, can we make this a constexpr 
> constructor/collection of some sort and instantiate it inline?  
> Instead of this initialization, can we make this a constexpr 
> constructor/collection of some sort and instantiate it inline?  

I don't have a good idea because it relies on Context to get some types like 
const char *



Comment at: clang/lib/CodeGen/CGBuiltin.cpp:2071
+  if (Types.find(Type) == Types.end())
+return Types[Context.VoidPtrTy];
+  return Types[Type];

erichkeane wrote:
> When can we hit this case?  Does this mean that any types we dont understand 
> we are just treating as void ptrs?  
The previous version treated it as a void *ptr for unsupported types, such as 
arrays (before this patch) and __int128. This is the case i saw in an issue. I 
also think that for unsupported types, we should generate a clearer message 
saying "This type is temporarily not supported", do you have any good ideas?



Comment at: clang/lib/CodeGen/CGBuiltin.cpp:2119
+  // But if the array length is constant, we can suppress that.
+  if (llvm::ConstantInt *CL = dyn_cast(Length)) {
+// ...and if it's constant zero, we can just skip the entire thing.

erichkeane wrote:
> This branch doesn't seem right?  We are in a `ConstantArrayType`, which means 
> we DEFINITELY know the length.  Additionally, `Length` is constructed 
> directly from a `llvm::ConstantInt::get`, so we already know this answer.
I agree with you



Comment at: clang/lib/CodeGen/CGBuiltin.cpp:2138
+
+  if (CheckZeroLength) {
+llvm::Value *isEmpty =

erichkeane wrote:
> We already know if this is a 'zero' length array, we should probably just not 
> emit IR at all in that case.
Good idea!



Comment at: clang/lib/CodeGen/CGBuiltin.cpp:2204
+  if (inArray) {
+GString = CGF.Builder.CreateGlobalStringPtr("{\n");
+  } else {

erichkeane wrote:
> instead of `inArray` you probably should have something a little more 
> descriptive here, it seems what you really need is something like, 
> `includeTypeName`, preferably as an enum to make it more clear at the caller 
> side.
I think that's a good idea for clarity.



Comment at: clang/lib/CodeGen/CGBuiltin.cpp:2257
+
+if (CanonicalType->isConstantArrayType()) {
+  GString = CGF.Builder.CreateGlobalStringPtr(

erichkeane wrote:
> Do we want to do anything for the OTHER array types?  
I understand struct fields must have a constant size, and I see" 'variable 
length array in structure' extension will never be supported" from clang's 
diagnostics



Comment at: clang/lib/CodeGen/CGBuiltin.cpp:2827
 LValue RecordLV = MakeAddrLValue(RecordPtr, Arg0Type, Arg0Align);
-Value *Res = dumpRecord(*this, Arg0Type, RecordLV, Arg0Align,
+Value *Res = dumpRecord(*this, Arg0Type, RecordLV, Arg0Align, false,
 {LLVMFuncType, Func}, 0);

erichkeane wrote:
> Typically we do a comment to explain what bool parms mean.  Alternatively, if 
> instead you could create an enum of some sort that would better describe the 
> 'false', it wouldlikely be more helpful.  (same on 2252).
I agree!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122822

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

[PATCH] D122273: [clang][dataflow] Fix handling of base-class fields

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel updated this revision to Diff 419496.
ymandel added a comment.

address reviewer comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122273

Files:
  clang/include/clang/Analysis/FlowSensitive/StorageLocation.h
  clang/include/clang/Analysis/FlowSensitive/Value.h
  clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
  clang/unittests/Analysis/FlowSensitive/TransferTest.cpp

Index: clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
@@ -1009,6 +1009,150 @@
   });
 }
 
+TEST_F(TransferTest, DerivedBaseMemberClass) {
+  std::string Code = R"(
+class A {
+  int ADefault;
+private:
+  int APrivate;
+public:
+  int APublic;
+};
+
+class B : public A {
+  int BDefault;
+protected:
+  int BProtected;
+private:
+  int BPrivate;
+};
+
+void target() {
+  B Foo;
+  // [[p]]
+}
+  )";
+  runDataflow(
+  Code, [](llvm::ArrayRef<
+   std::pair>>
+   Results,
+   ASTContext ) {
+ASSERT_THAT(Results, ElementsAre(Pair("p", _)));
+const Environment  = Results[0].second.Env;
+
+const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
+ASSERT_THAT(FooDecl, NotNull());
+ASSERT_TRUE(FooDecl->getType()->isRecordType());
+
+// Derived-class fields.
+const FieldDecl *BDefaultDecl = nullptr;
+const FieldDecl *BProtectedDecl = nullptr;
+const FieldDecl *BPrivateDecl = nullptr;
+for (const FieldDecl *Field :
+ FooDecl->getType()->getAsRecordDecl()->fields()) {
+  if (Field->getNameAsString() == "BDefault") {
+BDefaultDecl = Field;
+  } else if (Field->getNameAsString() == "BProtected") {
+BProtectedDecl = Field;
+  } else if (Field->getNameAsString() == "BPrivate") {
+BPrivateDecl = Field;
+  } else {
+FAIL() << "Unexpected field: " << Field->getNameAsString();
+  }
+}
+ASSERT_THAT(BDefaultDecl, NotNull());
+ASSERT_THAT(BProtectedDecl, NotNull());
+ASSERT_THAT(BPrivateDecl, NotNull());
+
+// Base-class fields.
+const FieldDecl *ADefaultDecl = nullptr;
+const FieldDecl *APrivateDecl = nullptr;
+const FieldDecl *APublicDecl = nullptr;
+for (const clang::CXXBaseSpecifier  :
+ FooDecl->getType()->getAsCXXRecordDecl()->bases()) {
+  QualType BaseType = Base.getType();
+  ASSERT_TRUE(BaseType->isRecordType());
+  for (const FieldDecl *Field : BaseType->getAsRecordDecl()->fields()) {
+if (Field->getNameAsString() == "ADefault") {
+  ADefaultDecl = Field;
+} else if (Field->getNameAsString() == "APrivate") {
+  APrivateDecl = Field;
+} else if (Field->getNameAsString() == "APublic") {
+  APublicDecl = Field;
+} else {
+  FAIL() << "Unexpected field: " << Field->getNameAsString();
+}
+  }
+}
+ASSERT_THAT(ADefaultDecl, NotNull());
+ASSERT_THAT(APrivateDecl, NotNull());
+ASSERT_THAT(APublicDecl, NotNull());
+
+const auto *FooLoc = cast(
+Env.getStorageLocation(*FooDecl, SkipPast::None));
+const auto  = *cast(Env.getValue(*FooLoc));
+EXPECT_THAT(FooVal.getChild(*APublicDecl), NotNull());
+
+// Base-class fields.
+EXPECT_THAT(FooVal.getChild(*ADefaultDecl), IsNull());
+EXPECT_THAT(FooVal.getChild(*APrivateDecl), IsNull());
+EXPECT_THAT(FooVal.getChild(*APublicDecl), NotNull());
+
+// Derived-class fields.
+EXPECT_THAT(FooVal.getChild(*BDefaultDecl), NotNull());
+EXPECT_THAT(FooVal.getChild(*BProtectedDecl), NotNull());
+EXPECT_THAT(FooVal.getChild(*BPrivateDecl), NotNull());
+  });
+}
+
+TEST_F(TransferTest, DerivedBaseMemberStructDefault) {
+  std::string Code = R"(
+struct A {
+  int Bar;
+};
+struct B : public A {
+};
+
+void target() {
+  B Foo;
+  // [[p]]
+}
+  )";
+  runDataflow(
+  Code, [](llvm::ArrayRef<
+   std::pair>>
+   Results,
+   ASTContext ) {
+ASSERT_THAT(Results, ElementsAre(Pair("p", _)));
+const Environment  = Results[0].second.Env;
+
+const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
+ASSERT_THAT(FooDecl, NotNull());
+
+ASSERT_TRUE(FooDecl->getType()->isRecordType());
+const FieldDecl *BarDecl = nullptr;
+for (const clang::CXXBaseSpecifier  :
+ FooDecl->getType()->getAsCXXRecordDecl()->bases()) {
+  QualType BaseType 

[PATCH] D122830: [clang][dataflow] Add support for (built-in) (in)equality operators

[Gábor Horváth via Phabricator via cfe-commits]

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



Comment at: clang/lib/Analysis/FlowSensitive/Transfer.cpp:56
+Env.getValue(*RHSNorm, SkipPast::Reference)))
+  return Env.makeIff(*LHSValue, *RHSValue);
+

I think a possible alternative way to handle this is to do some sort of 
"unification" for the two values. The current approach is superior in the sense 
that solving and generating constraints are clearly separate. Also this might 
be better for generating useful error messages. In case the solver ever becomes 
a bottleneck, I wonder whether replacing this with a unification step would 
speed things up a bit. 
Although that would only work for the equality case.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122830

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

[PATCH] D119996: [safestack] Support safestack in stack size diagnostics

[Paul Kirth via Phabricator via cfe-commits]

paulkirth updated this revision to Diff 419493.
paulkirth added a comment.
Herald added a subscriber: pengfei.

Update tests.

- Dont rely on size of `int` based on platform
- Add checking to backend tests for warn-stack-size to ensure the behavior is 
consistant when safestack is enabled


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119996

Files:
  clang/test/Frontend/stack-usage-safestack.c
  llvm/include/llvm/CodeGen/MachineFrameInfo.h
  llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
  llvm/lib/CodeGen/MachineFunction.cpp
  llvm/lib/CodeGen/PrologEpilogInserter.cpp
  llvm/lib/CodeGen/SafeStack.cpp
  llvm/test/CodeGen/X86/warn-stack.ll
  llvm/test/Transforms/SafeStack/ARM/debug.ll

Index: llvm/test/Transforms/SafeStack/ARM/debug.ll
===
--- llvm/test/Transforms/SafeStack/ARM/debug.ll
+++ llvm/test/Transforms/SafeStack/ARM/debug.ll
@@ -10,8 +10,8 @@
 ; void Capture(char*x);
 ; void f() { char c[16]; Capture(c); }
 
-; CHECK: !35 = !DILocation(line: 3, column: 11, scope: !17, inlinedAt: !36)
-; CHECK: !36 = distinct !DILocation(line: 6, scope: !27)
+; CHECK: !36 = !DILocation(line: 3, column: 11, scope: !17, inlinedAt: !37)
+; CHECK: !37 = distinct !DILocation(line: 6, scope: !27)
 
 @addr = common local_unnamed_addr global i8*** null, align 4, !dbg !0
 
Index: llvm/test/CodeGen/X86/warn-stack.ll
===
--- llvm/test/CodeGen/X86/warn-stack.ll
+++ llvm/test/CodeGen/X86/warn-stack.ll
@@ -21,4 +21,17 @@
   ret void
 }
 
+; Ensure that warn-stack-size also considers the size of the unsafe stack.
+; With safestack enabled the machine stack size is well below 80, but the
+; combined stack size of the machine stack and unsafe stack will exceed the
+; warning threshold
+
+; CHECK: warning: stack frame size (120) exceeds limit (80) in function 'warn_safestack'
+define void @warn_safestack() nounwind ssp safestack "warn-stack-size"="80" {
+entry:
+  %buffer = alloca [80 x i8], align 1
+  %arraydecay = getelementptr inbounds [80 x i8], [80 x i8]* %buffer, i64 0, i64 0
+  call void @doit(i8* %arraydecay) nounwind
+  ret void
+}
 declare void @doit(i8*)
Index: llvm/lib/CodeGen/SafeStack.cpp
===
--- llvm/lib/CodeGen/SafeStack.cpp
+++ llvm/lib/CodeGen/SafeStack.cpp
@@ -48,6 +48,7 @@
 #include "llvm/IR/IntrinsicInst.h"
 #include "llvm/IR/Intrinsics.h"
 #include "llvm/IR/MDBuilder.h"
+#include "llvm/IR/Metadata.h"
 #include "llvm/IR/Module.h"
 #include "llvm/IR/Type.h"
 #include "llvm/IR/Use.h"
@@ -642,6 +643,13 @@
   // FIXME: no need to update BasePointer in leaf functions.
   unsigned FrameSize = alignTo(SSL.getFrameSize(), StackAlignment);
 
+  MDBuilder MDB(F.getContext());
+  SmallVector Data;
+  Data.push_back(MDB.createString("unsafe-stack-size"));
+  Data.push_back(MDB.createConstant(ConstantInt::get(Int32Ty, FrameSize)));
+  MDNode *MD = MDTuple::get(F.getContext(), Data);
+  F.setMetadata(LLVMContext::MD_annotation, MD);
+
   // Update shadow stack pointer in the function epilogue.
   IRB.SetInsertPoint(BasePointer->getNextNode());
 
Index: llvm/lib/CodeGen/PrologEpilogInserter.cpp
===
--- llvm/lib/CodeGen/PrologEpilogInserter.cpp
+++ llvm/lib/CodeGen/PrologEpilogInserter.cpp
@@ -283,6 +283,9 @@
 assert(!Failed && "Invalid warn-stack-size fn attr value");
 (void)Failed;
   }
+  if (MF.getFunction().hasFnAttribute(Attribute::SafeStack)) {
+StackSize += MFI.getUnsafeStackSize();
+  }
   if (StackSize > Threshold) {
 DiagnosticInfoStackSize DiagStackSize(F, StackSize, Threshold, DS_Warning);
 F.getContext().diagnose(DiagStackSize);
Index: llvm/lib/CodeGen/MachineFunction.cpp
===
--- llvm/lib/CodeGen/MachineFunction.cpp
+++ llvm/lib/CodeGen/MachineFunction.cpp
@@ -107,6 +107,27 @@
   llvm_unreachable("Invalid machine function property");
 }
 
+void setUnsafeStackSize(const Function , MachineFrameInfo ) {
+  if (!F.hasFnAttribute(Attribute::SafeStack))
+return;
+
+  auto *Existing =
+  dyn_cast_or_null(F.getMetadata(LLVMContext::MD_annotation));
+
+  if (!Existing || Existing->getNumOperands() != 2)
+return;
+
+  auto *MetadataName = "unsafe-stack-size";
+  if (auto  = Existing->getOperand(0)) {
+if (cast(N.get())->getString() == MetadataName) {
+  if (auto  = Existing->getOperand(1)) {
+auto Val = mdconst::extract(Op)->getZExtValue();
+FrameInfo.setUnsafeStackSize(Val);
+  }
+}
+  }
+}
+
 // Pin the vtable to this file.
 void MachineFunction::Delegate::anchor() {}
 
@@ -175,6 +196,8 @@
   /*ForcedRealign=*/CanRealignSP &&
   F.hasFnAttribute(Attribute::StackAlignment));
 
+  setUnsafeStackSize(F, *FrameInfo);
+
   if 

[PATCH] D122781: This patch aims to conform AMDGPUOpenMP driver sanitizer changes w.r.t HIPAMD toolchain.

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert added a comment.

Commit name and message should be revisited.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122781

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

[PATCH] D94942: [clangd] Add tweak for implementing abstract class

[Nathan James via Phabricator via cfe-commits]

njames93 added a comment.

In D94942#3419616 , @sammccall wrote:

> @njames93 I had completely forgotten about this when I attempted the same 
> thing early this year.
> I never finished it but wanted to share what I had in case it's useful: 
> http://reviews.llvm.org/D122827. Also happy to finish that sometime if you're 
> not keen on completing this.
>
> Some thoughts based on that:
>
> - I think it's worth handling both "implement pure-virtuals" and "override 
> virtuals" as the same tweak, choosing based on context. >90% of the work is 
> the same. (I think my patch gets detection wrong though).
> - we now have a library to pick insertion points in a class a bit more 
> declaratively
> - I think you can use getFinalOverriders to avoid a lot of work traversing 
> class hierarchies
> - if you're not using getReturnType().print(..., /*Placeholder=*/Declarator) 
> then functions-that-return-function-pointers will definitely be rendered 
> wrong :-)

I stopped working on this a while ago but happy to resume (I've cherry picked 
this to the custom clangd build I use and it definitely has some value)
However I think we need a discussion about the ideal interface for this as 
there is no perfect solution for each use case. 
For example where do we trigger the tweak, and do we only override the pure 
virtual functions or is there value to have overrides for all virtual functions.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94942

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

[PATCH] D122831: [OpenMP] Make the new offloading driver the default

[Joseph Huber via Phabricator via cfe-commits]

jhuber6 added a comment.

In D122831#3419768 , @jdoerfert wrote:

> I feel we should port some of the clang driver tests rather than running them 
> only for the old one.
> The old one is on its way out, so ...

I tried to keep it where it worked, most of the ones changed were for checking 
things like nvlink specifically. One problem is the test that does `-E` which 
doesn't work right now because we won't generate offloading code without 
hitting a compile phase on the host. Will need to restructure some stuff to get 
that to work but I'm not sure how high the priority is when save-temps works 
fine.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122831

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

[PATCH] D122104: [X86][regcall] Support passing / returning structures

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/lib/CodeGen/TargetInfo.cpp:3992
 --FreeIntRegs;
+  else if (NeededSSE && MaxVectorWidth > 0)
+FI.setMaxVectorWidth(MaxVectorWidth);

erichkeane wrote:
> erichkeane wrote:
> > This line here accesses NeededSSE while uninitialized.  Do you  need to 
> > initalize it to something on 3963?
> Fixed: 
> https://github.com/llvm/llvm-project/commit/2267549296dabfed31ce194bb124f7bdb91f74c5
> 
> Please confirm the fix.
I've fixed this in 2267549296dabfed31ce194bb124f7bdb91f74c5 as it broke many 
tests on Windows (but I don't think one of the community build bots noticed 
this, which suggests we may want to have a Debug Windows builder).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122104

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

[PATCH] D121560: [clang][Opt] Add NoArgUnusedWith to not warn for unused redundant options

[Alex Brachet via Phabricator via cfe-commits]

abrachet updated this revision to Diff 419491.
abrachet added a comment.

Rebase so patch applies


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

https://reviews.llvm.org/D121560

Files:
  clang-tools-extra/clangd/CompileCommands.cpp
  clang/include/clang/Driver/Options.h
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/DriverOptions.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/Driver/claim-unused.c
  lld/COFF/Driver.h
  lld/COFF/DriverUtils.cpp
  lld/ELF/Driver.h
  lld/ELF/DriverUtils.cpp
  lld/MachO/Driver.h
  lld/MachO/DriverUtils.cpp
  lld/MinGW/Driver.cpp
  lld/wasm/Driver.cpp
  lldb/tools/driver/Driver.cpp
  lldb/tools/lldb-server/lldb-gdbserver.cpp
  lldb/tools/lldb-vscode/lldb-vscode.cpp
  llvm/include/llvm/Option/OptParser.td
  llvm/include/llvm/Option/OptTable.h
  llvm/include/llvm/Option/Option.h
  llvm/lib/ToolDrivers/llvm-dlltool/DlltoolDriver.cpp
  llvm/lib/ToolDrivers/llvm-lib/LibDriver.cpp
  llvm/tools/dsymutil/dsymutil.cpp
  llvm/tools/llvm-cvtres/llvm-cvtres.cpp
  llvm/tools/llvm-cxxfilt/llvm-cxxfilt.cpp
  llvm/tools/llvm-lipo/llvm-lipo.cpp
  llvm/tools/llvm-ml/llvm-ml.cpp
  llvm/tools/llvm-mt/llvm-mt.cpp
  llvm/tools/llvm-nm/llvm-nm.cpp
  llvm/tools/llvm-objcopy/ObjcopyOptions.cpp
  llvm/tools/llvm-objdump/ObjdumpOptID.h
  llvm/tools/llvm-objdump/llvm-objdump.cpp
  llvm/tools/llvm-rc/llvm-rc.cpp
  llvm/tools/llvm-readobj/llvm-readobj.cpp
  llvm/tools/llvm-size/llvm-size.cpp
  llvm/tools/llvm-strings/llvm-strings.cpp
  llvm/tools/llvm-symbolizer/llvm-symbolizer.cpp
  llvm/tools/llvm-tli-checker/llvm-tli-checker.cpp
  llvm/unittests/Option/OptionMarshallingTest.cpp
  llvm/unittests/Option/OptionParsingTest.cpp
  llvm/utils/TableGen/OptParserEmitter.cpp

Index: llvm/utils/TableGen/OptParserEmitter.cpp
===
--- llvm/utils/TableGen/OptParserEmitter.cpp
+++ llvm/utils/TableGen/OptParserEmitter.cpp
@@ -258,6 +258,16 @@
   OS << "#endif // PREFIX\n\n";
 
   OS << "/\n";
+
+  OS << R"(
+#ifndef OPT_PREFIX
+#define OPT_PREFIX
+#endif
+
+#define CAT_(A, B) A ## B
+#define CAT(A, B) CAT_(A, B)
+#define GET_OPT(OPT) CAT(OPT_PREFIX, OPT)
+  )";
   OS << "// Groups\n\n";
   OS << "#ifdef OPTION\n";
   for (const Record  : llvm::make_pointee_range(Groups)) {
@@ -298,6 +308,9 @@
 OS << ", nullptr";
 
 // The option Values (unused for groups).
+OS << ", nullptr";
+
+// NoArgUnusedWith (unused for groups).
 OS << ", nullptr)\n";
   }
   OS << "\n";
@@ -388,6 +401,20 @@
   write_cstring(OS, R.getValueAsString("Values"));
 else
   OS << "nullptr";
+
+// List of flags who's presence should cause this flag to not warn if used.
+OS << ", ";
+std::vector List = R.getValueAsListOfDefs("NoArgUnusedWith");
+if (!List.size()) {
+  OS << "nullptr";
+  return;
+}
+OS << "((const unsigned *)&(unsigned []){";
+// First element is the length of the array.
+OS << List.size();
+for (Record *R : List)
+  OS << ", GET_OPT(" << R->getName() << ")";
+OS << "})";
   };
 
   auto IsMarshallingOption = [](const Record ) {
@@ -405,6 +432,11 @@
   OptsWithMarshalling.push_back();
   }
   OS << "#endif // OPTION\n";
+  OS << R"(#undef GET_OPT
+#undef CAT
+#undef CAT_
+#undef OPT_PREFIX
+)";
 
   auto CmpMarshallingOpts = [](const Record *const *A, const Record *const *B) {
 unsigned AID = (*A)->getID();
Index: llvm/unittests/Option/OptionParsingTest.cpp
===
--- llvm/unittests/Option/OptionParsingTest.cpp
+++ llvm/unittests/Option/OptionParsingTest.cpp
@@ -18,7 +18,7 @@
 enum ID {
   OPT_INVALID = 0, // This is not an option ID.
 #define OPTION(PREFIX, NAME, ID, KIND, GROUP, ALIAS, ALIASARGS, FLAGS, PARAM,  \
-   HELPTEXT, METAVAR, VALUES)  \
+   HELPTEXT, METAVAR, VALUES, UNUSEDWITH)  \
   OPT_##ID,
 #include "Opts.inc"
   LastOption
@@ -37,9 +37,9 @@
 
 static const OptTable::Info InfoTable[] = {
 #define OPTION(PREFIX, NAME, ID, KIND, GROUP, ALIAS, ALIASARGS, FLAGS, PARAM,  \
-   HELPTEXT, METAVAR, VALUES)  \
+   HELPTEXT, METAVAR, VALUES, UNUSEDWITH)  \
   {PREFIX, NAME,  HELPTEXT,METAVAR, OPT_##ID,  Option::KIND##Class,\
-   PARAM,  FLAGS, OPT_##GROUP, OPT_##ALIAS, ALIASARGS, VALUES},
+   PARAM,  FLAGS, OPT_##GROUP, OPT_##ALIAS, ALIASARGS, VALUES, UNUSEDWITH},
 #include "Opts.inc"
 #undef OPTION
 };
Index: llvm/unittests/Option/OptionMarshallingTest.cpp
===
--- llvm/unittests/Option/OptionMarshallingTest.cpp
+++ llvm/unittests/Option/OptionMarshallingTest.cpp
@@ -18,9 +18,9 @@
 static const OptionWithMarshallingInfo MarshallingTable[] = {
 #define OPTION_WITH_MARSHALLING(

[PATCH] D122831: [OpenMP] Make the new offloading driver the default

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert added a comment.

I feel we should port some of the clang driver tests rather than running them 
only for the old one.
The old one is on its way out, so ...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122831

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

[PATCH] D122104: [X86][regcall] Support passing / returning structures

[Erich Keane via Phabricator via cfe-commits]

erichkeane added inline comments.



Comment at: clang/lib/CodeGen/TargetInfo.cpp:3992
 --FreeIntRegs;
+  else if (NeededSSE && MaxVectorWidth > 0)
+FI.setMaxVectorWidth(MaxVectorWidth);

erichkeane wrote:
> This line here accesses NeededSSE while uninitialized.  Do you  need to 
> initalize it to something on 3963?
Fixed: 
https://github.com/llvm/llvm-project/commit/2267549296dabfed31ce194bb124f7bdb91f74c5

Please confirm the fix.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122104

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

[clang] 2267549 - Fix the build after cd26190a10fceb6e1472fabcd9e1736f62f078c4

[Aaron Ballman via cfe-commits]

Author: Aaron Ballman
Date: 2022-03-31T12:03:53-04:00
New Revision: 2267549296dabfed31ce194bb124f7bdb91f74c5

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

LOG: Fix the build after cd26190a10fceb6e1472fabcd9e1736f62f078c4

These variables were being used uninitialized and it caused a
significant number of test failures on Windows.

Added: 


Modified: 
clang/lib/CodeGen/TargetInfo.cpp

Removed: 




diff  --git a/clang/lib/CodeGen/TargetInfo.cpp 
b/clang/lib/CodeGen/TargetInfo.cpp
index e1df6f505b17c..c5a031d5487cf 100644
--- a/clang/lib/CodeGen/TargetInfo.cpp
+++ b/clang/lib/CodeGen/TargetInfo.cpp
@@ -3960,7 +3960,7 @@ void X86_64ABIInfo::computeInfo(CGFunctionInfo ) const 
{
   // Keep track of the number of assigned registers.
   unsigned FreeIntRegs = IsRegCall ? 11 : 6;
   unsigned FreeSSERegs = IsRegCall ? 16 : 8;
-  unsigned NeededInt, NeededSSE, MaxVectorWidth = 0;
+  unsigned NeededInt = 0, NeededSSE = 0, MaxVectorWidth = 0;
 
   if (!::classifyReturnType(getCXXABI(), FI, *this)) {
 if (IsRegCall && FI.getReturnType()->getTypePtr()->isRecordType() &&



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

[clang-tools-extra] f43c4c5 - Revert "[clangd] IncludeCleaner: Add support for IWYU pragma private"

[Kirill Bobyrev via cfe-commits]

Author: Kirill Bobyrev
Date: 2022-03-31T17:59:52+02:00
New Revision: f43c4c5be29b4111bb953371b8ca83a4511fb1c1

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

LOG: Revert "[clangd] IncludeCleaner: Add support for IWYU pragma private"

This reverts commit 4cb38bfe76b7ef157485338623c931d04d17b958.

Awkwardly enough, this builds Windows buildbots:

http://45.33.8.238/win/55402/step_9.txt

It is yet unclear why this is happening but I will need more time to
diagnose the issue.

Added: 


Modified: 
clang-tools-extra/clangd/IncludeCleaner.cpp
clang-tools-extra/clangd/IncludeCleaner.h
clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/IncludeCleaner.cpp 
b/clang-tools-extra/clangd/IncludeCleaner.cpp
index 14cc3409cae49..04dbf12410cf7 100644
--- a/clang-tools-extra/clangd/IncludeCleaner.cpp
+++ b/clang-tools-extra/clangd/IncludeCleaner.cpp
@@ -12,7 +12,6 @@
 #include "ParsedAST.h"
 #include "Protocol.h"
 #include "SourceCode.h"
-#include "index/CanonicalIncludes.h"
 #include "support/Logger.h"
 #include "support/Trace.h"
 #include "clang/AST/ASTContext.h"
@@ -24,7 +23,6 @@
 #include "clang/Lex/Preprocessor.h"
 #include "clang/Tooling/Syntax/Tokens.h"
 #include "llvm/ADT/STLFunctionalExtras.h"
-#include "llvm/ADT/StringSet.h"
 #include "llvm/Support/FormatVariadic.h"
 #include "llvm/Support/Path.h"
 
@@ -305,10 +303,9 @@ ReferencedLocations findReferencedLocations(ParsedAST 
) {
  ());
 }
 
-ReferencedFiles findReferencedFiles(
-const ReferencedLocations , const SourceManager ,
-llvm::function_ref HeaderResponsible,
-llvm::function_ref(FileID)> UmbrellaHeader) {
+ReferencedFiles
+findReferencedFiles(const ReferencedLocations , const SourceManager ,
+llvm::function_ref HeaderResponsible) {
   std::vector Sorted{Locs.User.begin(), Locs.User.end()};
   llvm::sort(Sorted); // Group by FileID.
   ReferencedFilesBuilder Builder(SM);
@@ -327,55 +324,33 @@ ReferencedFiles findReferencedFiles(
   // non-self-contained FileIDs as used. Perform this on FileIDs rather than
   // HeaderIDs, as each inclusion of a non-self-contained file is distinct.
   llvm::DenseSet UserFiles;
-  llvm::StringSet<> PublicHeaders;
-  for (FileID ID : Builder.Files) {
+  for (FileID ID : Builder.Files)
 UserFiles.insert(HeaderResponsible(ID));
-if (auto PublicHeader = UmbrellaHeader(ID)) {
-  PublicHeaders.insert(*PublicHeader);
-}
-  }
 
   llvm::DenseSet StdlibFiles;
   for (const auto  : Locs.Stdlib)
 for (const auto  : Symbol.headers())
   StdlibFiles.insert(Header);
 
-  return {std::move(UserFiles), std::move(StdlibFiles),
-  std::move(PublicHeaders)};
+  return {std::move(UserFiles), std::move(StdlibFiles)};
 }
 
 ReferencedFiles findReferencedFiles(const ReferencedLocations ,
 const IncludeStructure ,
-const CanonicalIncludes ,
 const SourceManager ) {
-  return findReferencedFiles(
-  Locs, SM,
-  [, ](FileID ID) {
-return headerResponsible(ID, SM, Includes);
-  },
-  [, ](FileID ID) -> Optional {
-const FileEntry *Entry = SM.getFileEntryForID(ID);
-if (Entry) {
-  auto PublicHeader = CanonIncludes.mapHeader(Entry->getName());
-  if (!PublicHeader.empty()) {
-return PublicHeader;
-  }
-}
-return llvm::None;
-  });
+  return findReferencedFiles(Locs, SM, [, ](FileID ID) {
+return headerResponsible(ID, SM, Includes);
+  });
 }
 
 std::vector
 getUnused(ParsedAST ,
-  const llvm::DenseSet ,
-  const llvm::StringSet<> ) {
+  const llvm::DenseSet ) {
   trace::Span Tracer("IncludeCleaner::getUnused");
   std::vector Unused;
   for (const Inclusion  : AST.getIncludeStructure().MainFileIncludes) {
 if (!MFI.HeaderID)
   continue;
-if (ReferencedPublicHeaders.contains(MFI.Written))
-  continue;
 auto IncludeID = static_cast(*MFI.HeaderID);
 bool Used = ReferencedFiles.contains(IncludeID);
 if (!Used && !mayConsiderUnused(MFI, AST)) {
@@ -425,12 +400,11 @@ std::vector 
computeUnusedIncludes(ParsedAST ) {
   const auto  = AST.getSourceManager();
 
   auto Refs = findReferencedLocations(AST);
-  auto ReferencedFiles =
-  findReferencedFiles(Refs, AST.getIncludeStructure(),
-  AST.getCanonicalIncludes(), AST.getSourceManager());
+  auto ReferencedFileIDs = findReferencedFiles(Refs, AST.getIncludeStructure(),
+   AST.getSourceManager());
   auto ReferencedHeaders =
-  translateToHeaderIDs(ReferencedFiles,