[PATCH] D121992: [Clang] [Driver] Add option to set alternative toolchain path

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

To add more why I think the current semantics may need more discussion before 
we quickly commit to such a driver option:

- interaction with --dyld-prefix: --sysroot does not affect the fallback 
--dyld-prefix. It seems even less appropriate for --overlay-platform-toolchain 
to affect the fallback --dyld-prefix.

If you intend to overlay ld.so, you'll necessarily overlay libc, then --sysroot 
seems just unneeded at all.

- interaction with --gcc-toolchain: I am a bit unclear we still want the tricky 
GCC installation detection after --overlay-platform-toolchain is specified. Do 
you propose that both will add include and library search paths?
- -B: in gcc, when -B prefix specifies a directory, GCC adds $prefix/include to 
the include search directory. Clang does not do this right now. I think adding 
it may be an alternative approach to introducing the new option.

The currently picked rules may be suitable for 
https://github.com/advancetoolchain/advance-toolchain, but could be arbitrary 
for many other use cases.
Have you considered putting some options into a configuration file 
 and using 
`--config`?

I understand that you probably have some short-term goal to make somethings 
done, but as I mentioned, there might be some process issue here.
This significant new feature very quickly landed without other driver folks 
possibly had a chance to chime in.
(FWIW I decided to subscribe all `clang/lib/Driver` patches since I care about 
this area.)
I very rarely do this but I think it is probably cleaner to revert this patch 
and discuss it more carefully. I am happy to help you achieve your goal. It's 
possible that we may still need a driver option.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121992

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

[PATCH] D122231: [clang][dataflow] Add support for `value_or` in a comparison.

[Stanislav Gatev via Phabricator via cfe-commits]

sgatev accepted this revision.
sgatev added inline comments.



Comment at: 
clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp:275
+  State.Env.getValue(*ValueOrPredExpr, SkipPast::None));
+  if (ExprValue == nullptr) {
+auto &ExprLoc = State.Env.createStorageLocation(*ValueOrPredExpr);

ymandel wrote:
> sgatev wrote:
> > Why do this conditionally? I think we should set a value regardless of 
> > whether another model has already done so.
> Why? I figured we're agnostic to the underlying value, and only care about 
> relating it via the implication. We're setting it only so we have something 
> to anchor that implication on.  If we always set it, then we're erasing the 
> information from another model.
Nevermind. I probably didn't follow carefully around all the comment blocks and 
thought that `addToFlowCondition` also happens conditionally. The current 
approach looks good to me.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122231

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

[PATCH] D121992: [Clang] [Driver] Add option to set alternative toolchain path

[Qiu Chaofan via Phabricator via cfe-commits]

qiucf added a comment.

In D121992#3418443 , @MaskRay wrote:

> To add more why I think the current semantics may need more discussion before 
> we quickly commit to such a driver option:
>
> - interaction with --dyld-prefix: --sysroot does not affect the fallback 
> --dyld-prefix. It seems even less appropriate for 
> --overlay-platform-toolchain to affect the fallback --dyld-prefix.
>
> If you intend to overlay ld.so, you'll necessarily overlay libc, then 
> --sysroot seems just unneeded at all.
>
> - interaction with --gcc-toolchain: I am a bit unclear we still want the 
> tricky GCC installation detection after --overlay-platform-toolchain is 
> specified. Do you propose that both will add include and library search paths?
> - -B: in gcc, when -B prefix specifies a directory, GCC adds $prefix/include 
> to the include search directory. Clang does not do this right now. I think 
> adding it may be an alternative approach to introducing the new option.
>
> The currently picked rules may be suitable for 
> https://github.com/advancetoolchain/advance-toolchain, but could be arbitrary 
> for many other use cases.
> Have you considered putting some options into a configuration file 
>  and using 
> `--config`?
>
> I understand that you probably have some short-term goal to make somethings 
> done, but as I mentioned, there might be some process issue here.
> This significant new feature very quickly landed without other driver folks 
> possibly had a chance to chime in.
> (FWIW I decided to subscribe all `clang/lib/Driver` patches since I care 
> about this area.)
> I very rarely do this but I think it is probably cleaner to revert this patch 
> and discuss it more carefully. I am happy to help you achieve your goal. It's 
> possible that we may still need a driver option.

Thanks for further information. I agree that we should not change to break the 
consistent behavior in such short period of time. It's reasonable for 
`--gcc-toolchain` to not add `include` into path (since driver only expects it 
to have GCC). I also saw GCC's different behavior on `-B`. We may need to fix 
it, but that's beyond this patch's scope.

It's okay to me to compose 'orthogonal' options into config file when 
available. Anyway, I'd like to revert this commit since there're comments not 
addressed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121992

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

[clang] e1b8543 - Revert "[Clang] Add option to set alternative toolchain path"

[Qiu Chaofan via cfe-commits]

Author: Qiu Chaofan
Date: 2022-03-31T15:58:01+08:00
New Revision: e1b85430e98f243a32156617a54f3f01fd74fd8e

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

LOG: Revert "[Clang] Add option to set alternative toolchain path"

--overlay-platform-toolchain inserts a whole new toolchain path with
higher priority than system default, which could be achieved by
composing smaller options. We need to figure out alternative solution
and what is missing among these basic options.

Added: 


Modified: 
clang/docs/ClangCommandLineReference.rst
clang/include/clang/Driver/Driver.h
clang/include/clang/Driver/Options.td
clang/lib/Driver/Driver.cpp
clang/lib/Driver/ToolChains/Gnu.cpp
clang/lib/Driver/ToolChains/Linux.cpp

Removed: 
clang/test/Driver/Inputs/powerpc64le-linux-gnu-tree/gcc-11.2.0/include/.keep
clang/test/Driver/Inputs/powerpc64le-linux-gnu-tree/gcc-11.2.0/lib64/.keep
clang/test/Driver/Inputs/powerpc64le-linux-gnu-tree/gcc-8.3.0/include/.keep

clang/test/Driver/Inputs/powerpc64le-linux-gnu-tree/gcc-8.3.0/lib/gcc/powerpc64le-linux-gnu/8.3.0/.keep
clang/test/Driver/Inputs/powerpc64le-linux-gnu-tree/gcc-8.3.0/lib64/.keep
clang/test/Driver/overlay-toolchain.cpp



diff  --git a/clang/docs/ClangCommandLineReference.rst 
b/clang/docs/ClangCommandLineReference.rst
index d226ab75b3288..9d097ccae6aab 100644
--- a/clang/docs/ClangCommandLineReference.rst
+++ b/clang/docs/ClangCommandLineReference.rst
@@ -515,10 +515,6 @@ CUDA offloading device architecture (e.g. sm\_35), or HIP 
offloading target ID i
 
 Specify comma-separated list of offloading target triples (CUDA and HIP only)
 
-.. option:: --overlay-platform-toolchain=
-
-Specify a toolchain with higher priority than sysroot in search paths.
-
 .. option:: -p, --profile
 
 .. option:: -pagezero\_size

diff  --git a/clang/include/clang/Driver/Driver.h 
b/clang/include/clang/Driver/Driver.h
index 50aa7bea8f6d5..6f24f649ea544 100644
--- a/clang/include/clang/Driver/Driver.h
+++ b/clang/include/clang/Driver/Driver.h
@@ -149,9 +149,6 @@ class Driver {
   typedef SmallVector prefix_list;
   prefix_list PrefixDirs;
 
-  /// Alternative toolchain path used prior to sysroot.
-  std::string OverlayToolChainPath;
-
   /// sysroot, if present
   std::string SysRoot;
 

diff  --git a/clang/include/clang/Driver/Options.td 
b/clang/include/clang/Driver/Options.td
index 64e4f568ed67b..2dc28bab08ca4 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -4182,8 +4182,6 @@ def _output_class_directory_EQ : Joined<["--"], 
"output-class-directory=">, Alia
 def _output_class_directory : Separate<["--"], "output-class-directory">, 
Alias;
 def _output_EQ : Joined<["--"], "output=">, Alias;
 def _output : Separate<["--"], "output">, Alias;
-def _overlay_platform_toolchain_EQ : Joined<["--"], 
"overlay-platform-toolchain=">;
-def _overlay_platform_toolchain : Separate<["--"], 
"overlay-platform-toolchain">, Alias<_overlay_platform_toolchain_EQ>;
 def _param : Separate<["--"], "param">, Group;
 def _param_EQ : Joined<["--"], "param=">, Alias<_param>;
 def _precompile : Flag<["--"], "precompile">, Flags<[NoXarchOption]>,

diff  --git a/clang/lib/Driver/Driver.cpp b/clang/lib/Driver/Driver.cpp
index dd89121e671e5..0c79cc7589e19 100644
--- a/clang/lib/Driver/Driver.cpp
+++ b/clang/lib/Driver/Driver.cpp
@@ -1208,11 +1208,6 @@ Compilation *Driver::BuildCompilation(ArrayRef ArgList) {
   CompilerPath = Split.second;
 }
   }
-  if (const Arg *A =
-  Args.getLastArg(options::OPT__overlay_platform_toolchain_EQ)) {
-OverlayToolChainPath = A->getValue();
-DyldPrefix = A->getValue();
-  }
   if (const Arg *A = Args.getLastArg(options::OPT__sysroot_EQ))
 SysRoot = A->getValue();
   if (const Arg *A = Args.getLastArg(options::OPT__dyld_prefix_EQ))

diff  --git a/clang/lib/Driver/ToolChains/Gnu.cpp 
b/clang/lib/Driver/ToolChains/Gnu.cpp
index 7164f42170a8c..bb3cba6dc4f77 100644
--- a/clang/lib/Driver/ToolChains/Gnu.cpp
+++ b/clang/lib/Driver/ToolChains/Gnu.cpp
@@ -1867,10 +1867,6 @@ static llvm::StringRef getGCCToolchainDir(const ArgList 
&Args,
   if (A)
 return A->getValue();
 
-  if (const Arg *X = Args.getLastArg(
-  clang::driver::options::OPT__overlay_platform_toolchain_EQ))
-return X->getValue();
-
   // If we have a SysRoot, ignore GCC_INSTALL_PREFIX.
   // GCC_INSTALL_PREFIX specifies the gcc installation for the default
   // sysroot and is likely not valid with a 
diff erent sysroot.

diff  --git a/clang/lib/Driver/ToolChains/Linux.cpp 
b/clang/lib/Driver/ToolChains/Linux.cpp
index d647246f05349..83cb41159de7e 100644
--- a/clang/lib/Driver/ToolChains/Linux.cpp
+++ b/clang/lib/Driver/ToolChains/Linux.cpp
@@ -260,14 +260,6 @@ Linux::Linux(co

[PATCH] D119720: [ARM] Pass for Cortex-A57 and Cortex-A72 Fused AES Erratum

[Dave Green via Phabricator via cfe-commits]

dmgreen added inline comments.



Comment at: llvm/lib/Target/ARM/ARMFixCortexA57AES1742098Pass.cpp:145
+  case ARM::VMVNq:
+return CondCodeIsAL(3);
+  // VMOV of 64-bit value between D registers (when condition = al)

Can/should all these use findFirstPredOperandIdx?

And is it worth checking for more instruction? Anything that sets a Q register, 
or is that too broad?



Comment at: llvm/lib/Target/ARM/ARMFixCortexA57AES1742098Pass.cpp:250
+// Early return if no instructions are the start of an AES Pair.
+if (!llvm::any_of(MBB.instrs(), isFirstAESPairInstr))
+  continue;

This needn't scan through checking for the instruction that the loop below is 
going to check for too.



Comment at: llvm/lib/Target/ARM/ARMFixCortexA57AES1742098Pass.cpp:306
+  // fixup at the start of the function.
+  LLVM_DEBUG(dbgs()
+ << "Fixup Planned: Live-In (with safe defining instrs): "

Can you explain more about the IsLiveIn && UnsafeCount==0 case. Am I 
understanding that correctly that it would be:
```
function(q0, ...)
  lotsofcode...
  q0 = load
  aes q0
```
Is there a better way to detect that the live-in doesn't matter in cases like 
that?



Comment at: llvm/lib/Target/ARM/ARMTargetMachine.cpp:585
 
+  addPass(createARMFixCortexA57AES1742098Pass());
+

Passes can't insert new instructions (or move things further apart) after 
ConstantIslandPass. The branches/constant pools it has created may go out of 
range of the instructions that use them. Would it be OK to move it before that?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119720

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

[PATCH] D122789: [compiler-rt] [scudo] Use -mcrc32 on x86 when available

[Michał Górny via Phabricator via cfe-commits]

mgorny created this revision.
mgorny added reviewers: kostyakozko, MaskRay, alekseyshl, tianqing, pengfei.
mgorny added a project: Sanitizers.
Herald added subscribers: StephenFan, cryptoad, dberris.
Herald added a project: All.
mgorny requested review of this revision.

Update the hardware CRC32 logic in SCudo to support using `-mcrc32`
instead of `-msse4.2`.  The CRC32 intrinsics use the former flag
in the newer compiler versions, e.g. in clang since 12fa608af44a 
.
With these compilers, passing `-msse4.2` is insufficient to enable
the instructions and causes build failures when `-march` does not enable
CRC32:

  
/var/tmp/portage/sys-libs/compiler-rt-sanitizers-14.0.0/work/compiler-rt/lib/scudo/scudo_crc32.cpp:20:10:
 error: always_inline function '_mm_crc32_u32' requires target feature 'crc32', 
but would be inlined into function 'computeHardwareCRC32' that is compiled 
without support for 'crc32'
return CRC32_INTRINSIC(Crc, Data);
   ^
  
/var/tmp/portage/sys-libs/compiler-rt-sanitizers-14.0.0/work/compiler-rt/lib/scudo/scudo_crc32.h:27:27:
 note: expanded from macro 'CRC32_INTRINSIC'
  #  define CRC32_INTRINSIC FIRST_32_SECOND_64(_mm_crc32_u32, _mm_crc32_u64)
^
  
/var/tmp/portage/sys-libs/compiler-rt-sanitizers-14.0.0/work/compiler-rt/lib/scudo/../sanitizer_common/sanitizer_platform.h:132:36:
 note: expanded from macro 'FIRST_32_SECOND_64'
  #  define FIRST_32_SECOND_64(a, b) (a)
 ^
  1 error generated.

For backwards compatibility, use `-mcrc32` when available and fall back
to `-msse4.2`.  The `` header remains in use as it still
works and is compatible with GCC, while clang's ``
is not.

Originally reported in https://bugs.gentoo.org/835870.


https://reviews.llvm.org/D122789

Files:
  compiler-rt/cmake/config-ix.cmake
  compiler-rt/lib/scudo/CMakeLists.txt
  compiler-rt/lib/scudo/scudo_allocator.cpp
  compiler-rt/lib/scudo/scudo_crc32.cpp
  compiler-rt/lib/scudo/scudo_crc32.h
  compiler-rt/lib/scudo/standalone/CMakeLists.txt
  compiler-rt/lib/scudo/standalone/checksum.h
  compiler-rt/lib/scudo/standalone/chunk.h
  compiler-rt/lib/scudo/standalone/crc32_hw.cpp

Index: compiler-rt/lib/scudo/standalone/crc32_hw.cpp
===
--- compiler-rt/lib/scudo/standalone/crc32_hw.cpp
+++ compiler-rt/lib/scudo/standalone/crc32_hw.cpp
@@ -10,10 +10,10 @@
 
 namespace scudo {
 
-#if defined(__SSE4_2__) || defined(__ARM_FEATURE_CRC32)
+#if defined(__CRC32__) || defined(__SSE4_2__) || defined(__ARM_FEATURE_CRC32)
 u32 computeHardwareCRC32(u32 Crc, uptr Data) {
   return static_cast(CRC32_INTRINSIC(Crc, Data));
 }
-#endif // defined(__SSE4_2__) || defined(__ARM_FEATURE_CRC32)
+#endif // defined(__CRC32__) || defined(__SSE4_2__) || defined(__ARM_FEATURE_CRC32)
 
 } // namespace scudo
Index: compiler-rt/lib/scudo/standalone/chunk.h
===
--- compiler-rt/lib/scudo/standalone/chunk.h
+++ compiler-rt/lib/scudo/standalone/chunk.h
@@ -25,7 +25,7 @@
   // as opposed to only for crc32_hw.cpp. This means that other hardware
   // specific instructions were likely emitted at other places, and as a result
   // there is no reason to not use it here.
-#if defined(__SSE4_2__) || defined(__ARM_FEATURE_CRC32)
+#if defined(__CRC32__) || defined(__SSE4_2__) || defined(__ARM_FEATURE_CRC32)
   u32 Crc = static_cast(CRC32_INTRINSIC(Seed, Value));
   for (uptr I = 0; I < ArraySize; I++)
 Crc = static_cast(CRC32_INTRINSIC(Crc, Array[I]));
@@ -42,7 +42,7 @@
   Checksum = computeBSDChecksum(Checksum, Array[I]);
 return Checksum;
   }
-#endif // defined(__SSE4_2__) || defined(__ARM_FEATURE_CRC32)
+#endif // defined(__CRC32__) || defined(__SSE4_2__) || defined(__ARM_FEATURE_CRC32)
 }
 
 namespace Chunk {
Index: compiler-rt/lib/scudo/standalone/checksum.h
===
--- compiler-rt/lib/scudo/standalone/checksum.h
+++ compiler-rt/lib/scudo/standalone/checksum.h
@@ -12,12 +12,13 @@
 #include "internal_defs.h"
 
 // Hardware CRC32 is supported at compilation via the following:
-// - for i386 & x86_64: -msse4.2
+// - for i386 & x86_64: -mcrc32 (earlier: -msse4.2)
 // - for ARM & AArch64: -march=armv8-a+crc or -mcrc
 // An additional check must be performed at runtime as well to make sure the
 // emitted instructions are valid on the target host.
 
-#ifdef __SSE4_2__
+#if defined(__CRC32__) || defined(__SSE4_2__)
+// NB: clang has  but GCC does not
 #include 
 #define CRC32_INTRINSIC FIRST_32_SECOND_64(_mm_crc32_u32, _mm_crc32_u64)
 #endif
Index: compiler-rt/lib/scudo/standalone/CMakeLists.txt
===
--- compiler-rt/lib/scudo/standalone/CMakeLists.txt
+++ compiler-rt/lib/scudo/standalone/CMakeLists.txt
@@ -97,8 +97,11 @@
   string_utils.cpp
   )
 
-# Enable th

[PATCH] D119136: [clang] Implement Change scope of lambda trailing-return-type

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu added a comment.

I've just skimmed over the patch and it is a little bit hard to understand for 
people who don't have a strong background. Is it possible to split this one 
into several parts?




Comment at: clang/lib/Sema/Scope.cpp:72
+  // Lambdas have an extra prototype scope that doesn't add any depth
+  if (flags & FunctionPrototypeScope && !(flags & LambdaScope))
+PrototypeDepth++;

This looks like:



Comment at: clang/lib/Sema/SemaExpr.cpp:18242
 
+bool CheckCaptureUseBeforeLambdaQualifiers(Sema &S, VarDecl *Var,
+   SourceLocation ExprLoc,

If this is only used in current file, we should mark it as static or put in 
anonymous namespace. And I think this lacks a comment


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119136

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

[PATCH] D119720: [ARM] Pass for Cortex-A57 and Cortex-A72 Fused AES Erratum

[Sam Elliott via Phabricator via cfe-commits]

lenary added a comment.

Thanks for the review. Lots of comments inline, hopefully Phab doesn't mangle 
the large one.




Comment at: llvm/lib/Target/ARM/ARMFixCortexA57AES1742098Pass.cpp:145
+  case ARM::VMVNq:
+return CondCodeIsAL(3);
+  // VMOV of 64-bit value between D registers (when condition = al)

dmgreen wrote:
> Can/should all these use findFirstPredOperandIdx?
> 
> And is it worth checking for more instruction? Anything that sets a Q 
> register, or is that too broad?
`findFirstPredOperandIdx` doesn't work as lots of these instructions are not 
marked `isPredicable` in the tablegen. I'm not sure if we want to solve that in 
this work, or as a follow-up (I'd lean towards follow-up).

I believe "anything that sets a Q register" is too broad, as we model 
subregister insertion as setting the the whole register in LLVM, but I'm not 
sure that micro-architecturally they are actually doing that. This is why I've 
tried to only add 64- and 128-bit setting instructions rather than ones that 
are less wide. Originally I also included the `VMOVv2f32` instructions that are 
now at the bottom of this switch, but I felt that might have been too risky.



Comment at: llvm/lib/Target/ARM/ARMFixCortexA57AES1742098Pass.cpp:250
+// Early return if no instructions are the start of an AES Pair.
+if (!llvm::any_of(MBB.instrs(), isFirstAESPairInstr))
+  continue;

dmgreen wrote:
> This needn't scan through checking for the instruction that the loop below is 
> going to check for too.
Ack. Will remove. I think this is vestigal from a previous (unshared) version 
of the patch which was doing something more complex in the loop.



Comment at: llvm/lib/Target/ARM/ARMFixCortexA57AES1742098Pass.cpp:306
+  // fixup at the start of the function.
+  LLVM_DEBUG(dbgs()
+ << "Fixup Planned: Live-In (with safe defining instrs): "

dmgreen wrote:
> Can you explain more about the IsLiveIn && UnsafeCount==0 case. Am I 
> understanding that correctly that it would be:
> ```
> function(q0, ...)
>   lotsofcode...
>   q0 = load
>   aes q0
> ```
> Is there a better way to detect that the live-in doesn't matter in cases like 
> that?
I don't believe there is, and this comes down to issues with the 
`RDA.getGlobalReachingDefs` implementation, which I want to fix/enhance, but in 
a follow-up patch.

To start with, this is actually not a problem, as the pass is intended to be 
conservative, and we know the clobbers are a no-op at the architectural level 
(we insert them for their micro-architectural effects). So code will still do 
the right thing, but maybe with a little too much overhead in the case you 
showed.

However, this is necessary in some other cases, such as:

```
function(q0)
   code
   conditional branch to L2
L1:
   q0 = safe_op(…)
   branch to L3
L2:
   code without update to q0
L3:
   aes q0
```

In this case, `AllDefs` is a set containing one single defining instruction for 
Q0, because there is only one within the function (which is all that 
`RDA.getGlobalReachingDefs` can report instructions for).

But in my example, we *need* to protect q0 on the other paths, because the safe 
definitions of q0, when considered as a set, do not entirely dominate the AES 
use of q0 (this is slightly stretching the conventional definition of 
dominance, but think of this as "there exists a path from entry to the aes, 
which does not contain any of the safe instructions". Sadly, MachineDominance 
doesn't allow us to make this kind of query either!).

In this case though, it is safe to insert the protection at function entry, 
because that will (by definition) dominate all the AES uses, and the protection 
doesn't need to be dominated by the safe definitions, as we know they're safe.

I intend to follow-up this initial patch with an enhancement to 
ReachingDefAnalysis which will also provide the information that you have a set 
of defs inside the function, and also you're live-in, as this is required info 
for any conservative pass using the ReachingDefAnalysis. I felt, however, that 
given the pass is safe as-is, it was good to proceed without this enhancement.







Comment at: llvm/lib/Target/ARM/ARMTargetMachine.cpp:585
 
+  addPass(createARMFixCortexA57AES1742098Pass());
+

dmgreen wrote:
> Passes can't insert new instructions (or move things further apart) after 
> ConstantIslandPass. The branches/constant pools it has created may go out of 
> range of the instructions that use them. Would it be OK to move it before 
> that?
TIL. I'll add a comment about the constant island pass as well.

Should I also look at the restrictions on the Branch Targets pass? I can 
imagine we also don't want to separate instructions once we've calculated their 
targets locally?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D11972

[PATCH] D119544: Deferred Concept Instantiation Implementation

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu added a comment.

I am interested in this one. But it is absolutely not easy to understand...




Comment at: clang/include/clang/Sema/Sema.h:6991-6993
   /// \param TemplateArgs the list of template arguments to substitute into the
-  /// constraint expression.
+  /// constraint expression, which is multi-level since we have to substitute
+  /// ALL levels for the purposes of checking.

Now the comment is not precise.



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

I think this one need comment too. What's the difference with the above one?



Comment at: clang/lib/Sema/SemaConcept.cpp:167
+  return LHSRes.isUsable() ? BO.recreateBinOp(S, LHSRes) : ExprEmpty();
+// return BO.recreateBinOp(S, LHSRes);
 

We should delete this one.



Comment at: clang/lib/Sema/SemaConcept.cpp:177
+  return LHSRes.isUsable() ? BO.recreateBinOp(S, LHSRes) : ExprEmpty();
+// return BO.recreateBinOp(S, LHSRes);
 

ditto



Comment at: clang/lib/Sema/SemaConcept.cpp:429-435
+if (TemplateArgs) {
+  MultiLevelTemplateArgumentList JustTemplArgs(
+  *FD->getTemplateSpecializationArgs());
+  if (addInstantiatedParametersToScope(
+  FD, PrimaryTemplate->getTemplatedDecl(), Scope, JustTemplArgs))
+return {};
+}

Might you elaborate more on this. I am not sure about the intention.



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

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...


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] D119720: [ARM] Pass for Cortex-A57 and Cortex-A72 Fused AES Erratum

[Dave Green via Phabricator via cfe-commits]

dmgreen added inline comments.



Comment at: llvm/lib/Target/ARM/ARMFixCortexA57AES1742098Pass.cpp:200-207
+  case ARM::VMOVv2f32:
+  case ARM::VMOVv4f32:
+  case ARM::VMOVv2i32:
+  case ARM::VMOVv4i32:
+  case ARM::VMOVv4i16:
+  case ARM::VMOVv8i16:
+  case ARM::VMOVv8i8:

Are these vmov of an immediate? Are they not safe?

I was expecting it to be the lanes sets (VSETLNi8) and other scalar 
instructions that were unsafe.



Comment at: llvm/lib/Target/ARM/ARMFixCortexA57AES1742098Pass.cpp:306
+  // fixup at the start of the function.
+  LLVM_DEBUG(dbgs()
+ << "Fixup Planned: Live-In (with safe defining instrs): "

lenary wrote:
> dmgreen wrote:
> > Can you explain more about the IsLiveIn && UnsafeCount==0 case. Am I 
> > understanding that correctly that it would be:
> > ```
> > function(q0, ...)
> >   lotsofcode...
> >   q0 = load
> >   aes q0
> > ```
> > Is there a better way to detect that the live-in doesn't matter in cases 
> > like that?
> I don't believe there is, and this comes down to issues with the 
> `RDA.getGlobalReachingDefs` implementation, which I want to fix/enhance, but 
> in a follow-up patch.
> 
> To start with, this is actually not a problem, as the pass is intended to be 
> conservative, and we know the clobbers are a no-op at the architectural level 
> (we insert them for their micro-architectural effects). So code will still do 
> the right thing, but maybe with a little too much overhead in the case you 
> showed.
> 
> However, this is necessary in some other cases, such as:
> 
> ```
> function(q0)
>code
>conditional branch to L2
> L1:
>q0 = safe_op(…)
>branch to L3
> L2:
>code without update to q0
> L3:
>aes q0
> ```
> 
> In this case, `AllDefs` is a set containing one single defining instruction 
> for Q0, because there is only one within the function (which is all that 
> `RDA.getGlobalReachingDefs` can report instructions for).
> 
> But in my example, we *need* to protect q0 on the other paths, because the 
> safe definitions of q0, when considered as a set, do not entirely dominate 
> the AES use of q0 (this is slightly stretching the conventional definition of 
> dominance, but think of this as "there exists a path from entry to the aes, 
> which does not contain any of the safe instructions". Sadly, MachineDominance 
> doesn't allow us to make this kind of query either!).
> 
> In this case though, it is safe to insert the protection at function entry, 
> because that will (by definition) dominate all the AES uses, and the 
> protection doesn't need to be dominated by the safe definitions, as we know 
> they're safe.
> 
> I intend to follow-up this initial patch with an enhancement to 
> ReachingDefAnalysis which will also provide the information that you have a 
> set of defs inside the function, and also you're live-in, as this is required 
> info for any conservative pass using the ReachingDefAnalysis. I felt, 
> however, that given the pass is safe as-is, it was good to proceed without 
> this enhancement.
> 
> 
> 
> 
OK sounds good.



Comment at: llvm/lib/Target/ARM/ARMTargetMachine.cpp:585
 
+  addPass(createARMFixCortexA57AES1742098Pass());
+

lenary wrote:
> dmgreen wrote:
> > Passes can't insert new instructions (or move things further apart) after 
> > ConstantIslandPass. The branches/constant pools it has created may go out 
> > of range of the instructions that use them. Would it be OK to move it 
> > before that?
> TIL. I'll add a comment about the constant island pass as well.
> 
> Should I also look at the restrictions on the Branch Targets pass? I can 
> imagine we also don't want to separate instructions once we've calculated 
> their targets locally?
Yeah - It sounds like the BTI would need to remain as the first instruction in 
the block.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119720

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

[PATCH] D122525: [clang][ASTImporter] Fix an import error handling related bug.

[Gabor Marton via Phabricator via cfe-commits]

martong added a comment.

Nice catch!




Comment at: clang/lib/AST/ASTImporter.cpp:8789-8791
+// The import path contains child nodes first.
+// (Parent-child relationship is used here in sense of import
+// dependency.)

I think, this comment would be better to describe the declaration of 
`PrevFromDi`.



Comment at: clang/lib/AST/ASTImporter.cpp:8792-8794
+if (!isa(FromDi))
+  if (auto *FromDiDC = dyn_cast(FromDi))
+if (FromDiDC->containsDecl(PrevFromDi))

We should elevate this logic and the one in `ImportDeclContext` that uses 
`consumeError` into a common abstraction. I.e. making them closer to each other 
in a class.
Something like (draft, did not test it, might not compile):
```
class ErrorHandlingStrategy {
  static bool accumulateChildErrors(DeclContext*);
  static bool accumulateChildErrors(Decl *FromDi, Decl* Parent); // Use this 
one when traversing through the import path!
public:
  static Error handleDeclContextError(Error Err, DeclContext* FromDC) {
Error ChildErrors = Error::success();
if (Err && accumulateChildErrors(FromDC))
  ChildErrors =  joinErrors(std::move(ChildErrors), std::move(Err));
else
  consumeError(std::move(Err));
 return ChildErrors
  }
  
};
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122525

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

[PATCH] D122528: [clang][ASTImporter] Not using consumeError at failed import of in-class initializer.

[Gabor Marton via Phabricator via cfe-commits]

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

Thanks! Good catch!

(For inexperienced readers, let me explain why I accepted the patch: If there 
is an error then the imported Decl is marked as erroneous, i.e an Error object 
is associated with the imported Decl, thus users of the ASTImporter can react 
according to the set error.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122528

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

[PATCH] D120306: [clangd] IncludeCleaner: Add support for IWYU pragma private

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev updated this revision to Diff 419378.
kbobyrev marked 5 inline comments as done.
kbobyrev added a comment.

Address review comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120306

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

Index: clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
===
--- clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
+++ clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
@@ -9,6 +9,7 @@
 #include "Annotations.h"
 #include "IncludeCleaner.h"
 #include "SourceCode.h"
+#include "TestFS.h"
 #include "TestTU.h"
 #include "llvm/ADT/ScopeExit.h"
 #include "llvm/Testing/Support/SupportHelpers.h"
@@ -280,8 +281,9 @@
 
 ReferencedLocations Locs = findReferencedLocations(AST);
 EXPECT_THAT(Locs.Stdlib, ElementsAreArray(WantSyms));
-ReferencedFiles Files = findReferencedFiles(Locs, AST.getIncludeStructure(),
-AST.getSourceManager());
+ReferencedFiles Files =
+findReferencedFiles(Locs, AST.getIncludeStructure(),
+AST.getCanonicalIncludes(), AST.getSourceManager());
 EXPECT_THAT(Files.Stdlib, ElementsAreArray(WantHeaders));
   }
 }
@@ -392,8 +394,8 @@
   auto &SM = AST.getSourceManager();
   auto &Includes = AST.getIncludeStructure();
 
-  auto ReferencedFiles =
-  findReferencedFiles(findReferencedLocations(AST), Includes, SM);
+  auto ReferencedFiles = findReferencedFiles(
+  findReferencedLocations(AST), Includes, AST.getCanonicalIncludes(), SM);
   llvm::StringSet<> ReferencedFileNames;
   for (FileID FID : ReferencedFiles.User)
 ReferencedFileNames.insert(
@@ -412,7 +414,9 @@
   EXPECT_THAT(ReferencedHeaderNames, ElementsAre(testPath("macros.h")));
 
   // Sanity check.
-  EXPECT_THAT(getUnused(AST, ReferencedHeaders), IsEmpty());
+  EXPECT_THAT(
+  getUnused(AST, ReferencedHeaders, ReferencedFiles.SpelledUmbrellas),
+  IsEmpty());
 }
 
 TEST(IncludeCleaner, DistinctUnguardedInclusions) {
@@ -441,9 +445,9 @@
 
   ParsedAST AST = TU.build();
 
-  auto ReferencedFiles =
-  findReferencedFiles(findReferencedLocations(AST),
-  AST.getIncludeStructure(), AST.getSourceManager());
+  auto ReferencedFiles = findReferencedFiles(
+  findReferencedLocations(AST), AST.getIncludeStructure(),
+  AST.getCanonicalIncludes(), AST.getSourceManager());
   llvm::StringSet<> ReferencedFileNames;
   auto &SM = AST.getSourceManager();
   for (FileID FID : ReferencedFiles.User)
@@ -475,9 +479,9 @@
 
   ParsedAST AST = TU.build();
 
-  auto ReferencedFiles =
-  findReferencedFiles(findReferencedLocations(AST),
-  AST.getIncludeStructure(), AST.getSourceManager());
+  auto ReferencedFiles = findReferencedFiles(
+  findReferencedLocations(AST), AST.getIncludeStructure(),
+  AST.getCanonicalIncludes(), AST.getSourceManager());
   llvm::StringSet<> ReferencedFileNames;
   auto &SM = AST.getSourceManager();
   for (FileID FID : ReferencedFiles.User)
@@ -493,15 +497,31 @@
   TestTU TU;
   TU.Code = R"cpp(
 #include "behind_keep.h" // IWYU pragma: keep
+#include "public.h"
+
+void bar() { foo(); }
 )cpp";
   TU.AdditionalFiles["behind_keep.h"] = guard("");
+  TU.AdditionalFiles["public.h"] = guard("#include \"private.h\"");
+  TU.AdditionalFiles["private.h"] = guard(R"cpp(
+// IWYU pragma: private, include "public.h"
+void foo() {}
+  )cpp");
   ParsedAST AST = TU.build();
 
-  auto ReferencedFiles =
-  findReferencedFiles(findReferencedLocations(AST),
-  AST.getIncludeStructure(), AST.getSourceManager());
-  EXPECT_TRUE(ReferencedFiles.User.empty());
+  auto ReferencedFiles = findReferencedFiles(
+  findReferencedLocations(AST), AST.getIncludeStructure(),
+  AST.getCanonicalIncludes(), AST.getSourceManager());
+  EXPECT_EQ(ReferencedFiles.SpelledUmbrellas.size(), 1u);
+  EXPECT_EQ(ReferencedFiles.SpelledUmbrellas.begin()->getKey(), "\"public.h\"");
+  EXPECT_EQ(ReferencedFiles.User.size(), 2u);
+  EXPECT_TRUE(
+  ReferencedFiles.User.contains(AST.getSourceManager().getMainFileID()));
+  EXPECT_TRUE(
+  ReferencedFiles.User.contains(AST.getSourceManager().getMainFileID()));
   EXPECT_THAT(AST.getDiagnostics(), llvm::ValueIs(IsEmpty()));
+  auto Unused = computeUnusedIncludes(AST);
+  EXPECT_THAT(Unused, IsEmpty());
 }
 
 } // namespace
Index: clang-tools-extra/clangd/IncludeCleaner.h
===
--- clang-tools-extra/clangd/IncludeCleaner.h
+++ clang-tools-extra/clangd/IncludeCleaner.h
@@ -13,9 +13,6 @@
 /// to provide useful warnings in most popular scenarios but not 1:1 exact
 /// feature compatibility.
 ///

[PATCH] D122525: [clang][ASTImporter] Fix an import error handling related bug.

[Gabor Marton via Phabricator via cfe-commits]

martong added a comment.

Related: https://reviews.llvm.org/D112132


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122525

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

[PATCH] D121756: [clang-format] Clean up code looking for if statements NFC

[Owen Pan via Phabricator via cfe-commits]

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



Comment at: clang/lib/Format/TokenAnnotator.cpp:252-256
+} else if (OpeningParen.isConditionLParen(/*IncludeFor=*/false) ||
+   (OpeningParen.Previous &&
+OpeningParen.Previous->isOneOf(TT_BinaryOperator, tok::l_paren,
+   tok::comma,
+   tok::kw_static_assert))) {

I don't think this is NFC.
Before:
```
} else if (OpeningParen.Previous &&
   (OpeningParen.Previous->isOneOf(tok::kw_static_assert,
   tok::kw_while, tok::l_paren,
   tok::comma, tok::kw_if,
   TT_BinaryOperator) ||
OpeningParen.Previous->endsSequence(tok::kw_constexpr,
tok::kw_if) ||
OpeningParen.Previous->endsSequence(tok::identifier,
tok::kw_if))) {
```
After:
```
} else if ((OpeningParen.is(tok::l_paren) &&
OpeningParen.is(TT_ConditionLParen)) ||
   // PreviousNonComment = OpeningParen.getPreviousNonComment()
   (PreviousNonComment &&
PreviousNonComment->isOneOf(tok::kw_if, tok::kw_while,
tok::kw_switch, tok::kw_case,
tok::kw_constexpr)) ||
   (OpeningParen.Previous &&
OpeningParen.Previous->isOneOf(tok::kw_static_assert,
   tok::l_paren, tok::comma,
   TT_BinaryOperator))) {
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121756

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

[PATCH] D121445: [Clang][CSKY] Add the CSKY target and compiler driver

[Renato Golin via Phabricator via cfe-commits]

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

In D121445#3418195 , @zixuan-wu wrote:

> I have met this before because the downloading of patch will ignore empty 
> files. You can have a check that your apply does not contain new empty files 
> in multilib_csky_linux_sdk.

You are correct, it doesn't. :(

> For warnings, those are not introduced by this patch. I create another NFC to 
> remove unused variable and function.

Fair enough.

LGTM, thanks!


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

https://reviews.llvm.org/D121445

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

[PATCH] D122798: [clang][extract-api][NFC] Add documentation

[Daniel Grumberg via Phabricator via cfe-commits]

dang created this revision.
dang added a reviewer: zixuw.
Herald added a project: All.
dang requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Add struct level documentation for MacroDefinitionRecord.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D122798

Files:
  clang/include/clang/ExtractAPI/API.h


Index: clang/include/clang/ExtractAPI/API.h
===
--- clang/include/clang/ExtractAPI/API.h
+++ clang/include/clang/ExtractAPI/API.h
@@ -380,6 +380,7 @@
   virtual void anchor();
 };
 
+/// This holds information associated with macro definitions.
 struct MacroDefinitionRecord : APIRecord {
   MacroDefinitionRecord(StringRef Name, StringRef USR, PresumedLoc Loc,
 DeclarationFragments Declaration,


Index: clang/include/clang/ExtractAPI/API.h
===
--- clang/include/clang/ExtractAPI/API.h
+++ clang/include/clang/ExtractAPI/API.h
@@ -380,6 +380,7 @@
   virtual void anchor();
 };
 
+/// This holds information associated with macro definitions.
 struct MacroDefinitionRecord : APIRecord {
   MacroDefinitionRecord(StringRef Name, StringRef USR, PresumedLoc Loc,
 DeclarationFragments Declaration,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] 4cb38bf - [clangd] IncludeCleaner: Add support for IWYU pragma private

[Kirill Bobyrev via cfe-commits]

Author: Kirill Bobyrev
Date: 2022-03-31T12:49:52+02:00
New Revision: 4cb38bfe76b7ef157485338623c931d04d17b958

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

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

Reviewed By: sammccall

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

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 04dbf12410cf7..14cc3409cae49 100644
--- a/clang-tools-extra/clangd/IncludeCleaner.cpp
+++ b/clang-tools-extra/clangd/IncludeCleaner.cpp
@@ -12,6 +12,7 @@
 #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"
@@ -23,6 +24,7 @@
 #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"
 
@@ -303,9 +305,10 @@ ReferencedLocations findReferencedLocations(ParsedAST 
&AST) {
  &AST.getTokens());
 }
 
-ReferencedFiles
-findReferencedFiles(const ReferencedLocations &Locs, const SourceManager &SM,
-llvm::function_ref HeaderResponsible) {
+ReferencedFiles findReferencedFiles(
+const ReferencedLocations &Locs, const SourceManager &SM,
+llvm::function_ref HeaderResponsible,
+llvm::function_ref(FileID)> UmbrellaHeader) {
   std::vector Sorted{Locs.User.begin(), Locs.User.end()};
   llvm::sort(Sorted); // Group by FileID.
   ReferencedFilesBuilder Builder(SM);
@@ -324,33 +327,55 @@ findReferencedFiles(const ReferencedLocations &Locs, 
const SourceManager &SM,
   // 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;
-  for (FileID ID : Builder.Files)
+  llvm::StringSet<> PublicHeaders;
+  for (FileID ID : Builder.Files) {
 UserFiles.insert(HeaderResponsible(ID));
+if (auto PublicHeader = UmbrellaHeader(ID)) {
+  PublicHeaders.insert(*PublicHeader);
+}
+  }
 
   llvm::DenseSet StdlibFiles;
   for (const auto &Symbol : Locs.Stdlib)
 for (const auto &Header : Symbol.headers())
   StdlibFiles.insert(Header);
 
-  return {std::move(UserFiles), std::move(StdlibFiles)};
+  return {std::move(UserFiles), std::move(StdlibFiles),
+  std::move(PublicHeaders)};
 }
 
 ReferencedFiles findReferencedFiles(const ReferencedLocations &Locs,
 const IncludeStructure &Includes,
+const CanonicalIncludes &CanonIncludes,
 const SourceManager &SM) {
-  return findReferencedFiles(Locs, SM, [&SM, &Includes](FileID ID) {
-return headerResponsible(ID, SM, Includes);
-  });
+  return findReferencedFiles(
+  Locs, SM,
+  [&SM, &Includes](FileID ID) {
+return headerResponsible(ID, SM, Includes);
+  },
+  [&SM, &CanonIncludes](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;
+  });
 }
 
 std::vector
 getUnused(ParsedAST &AST,
-  const llvm::DenseSet &ReferencedFiles) {
+  const llvm::DenseSet &ReferencedFiles,
+  const llvm::StringSet<> &ReferencedPublicHeaders) {
   trace::Span Tracer("IncludeCleaner::getUnused");
   std::vector Unused;
   for (const Inclusion &MFI : 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)) {
@@ -400,11 +425,12 @@ std::vector 
computeUnusedIncludes(ParsedAST &AST) {
   const auto &SM = AST.getSourceManager();
 
   auto Refs = findReferencedLocations(AST);
-  auto ReferencedFileIDs = findReferencedFiles(Refs, AST.getIncludeStructure(),
-   AST.getSourceManager());
+  auto ReferencedFiles =
+  findReferencedFiles(Refs, AST.getIncludeStructure(),
+  AST.getCanonicalIncludes(), AST.getSourceManager());
   auto ReferencedHeaders =
- 

[PATCH] D120306: [clangd] IncludeCleaner: Add support for IWYU pragma private

[Kirill Bobyrev 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 rG4cb38bfe76b7: [clangd] IncludeCleaner: Add support for IWYU 
pragma private (authored by kbobyrev).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120306

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

Index: clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
===
--- clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
+++ clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
@@ -9,6 +9,7 @@
 #include "Annotations.h"
 #include "IncludeCleaner.h"
 #include "SourceCode.h"
+#include "TestFS.h"
 #include "TestTU.h"
 #include "llvm/ADT/ScopeExit.h"
 #include "llvm/Testing/Support/SupportHelpers.h"
@@ -280,8 +281,9 @@
 
 ReferencedLocations Locs = findReferencedLocations(AST);
 EXPECT_THAT(Locs.Stdlib, ElementsAreArray(WantSyms));
-ReferencedFiles Files = findReferencedFiles(Locs, AST.getIncludeStructure(),
-AST.getSourceManager());
+ReferencedFiles Files =
+findReferencedFiles(Locs, AST.getIncludeStructure(),
+AST.getCanonicalIncludes(), AST.getSourceManager());
 EXPECT_THAT(Files.Stdlib, ElementsAreArray(WantHeaders));
   }
 }
@@ -392,8 +394,8 @@
   auto &SM = AST.getSourceManager();
   auto &Includes = AST.getIncludeStructure();
 
-  auto ReferencedFiles =
-  findReferencedFiles(findReferencedLocations(AST), Includes, SM);
+  auto ReferencedFiles = findReferencedFiles(
+  findReferencedLocations(AST), Includes, AST.getCanonicalIncludes(), SM);
   llvm::StringSet<> ReferencedFileNames;
   for (FileID FID : ReferencedFiles.User)
 ReferencedFileNames.insert(
@@ -412,7 +414,9 @@
   EXPECT_THAT(ReferencedHeaderNames, ElementsAre(testPath("macros.h")));
 
   // Sanity check.
-  EXPECT_THAT(getUnused(AST, ReferencedHeaders), IsEmpty());
+  EXPECT_THAT(
+  getUnused(AST, ReferencedHeaders, ReferencedFiles.SpelledUmbrellas),
+  IsEmpty());
 }
 
 TEST(IncludeCleaner, DistinctUnguardedInclusions) {
@@ -441,9 +445,9 @@
 
   ParsedAST AST = TU.build();
 
-  auto ReferencedFiles =
-  findReferencedFiles(findReferencedLocations(AST),
-  AST.getIncludeStructure(), AST.getSourceManager());
+  auto ReferencedFiles = findReferencedFiles(
+  findReferencedLocations(AST), AST.getIncludeStructure(),
+  AST.getCanonicalIncludes(), AST.getSourceManager());
   llvm::StringSet<> ReferencedFileNames;
   auto &SM = AST.getSourceManager();
   for (FileID FID : ReferencedFiles.User)
@@ -475,9 +479,9 @@
 
   ParsedAST AST = TU.build();
 
-  auto ReferencedFiles =
-  findReferencedFiles(findReferencedLocations(AST),
-  AST.getIncludeStructure(), AST.getSourceManager());
+  auto ReferencedFiles = findReferencedFiles(
+  findReferencedLocations(AST), AST.getIncludeStructure(),
+  AST.getCanonicalIncludes(), AST.getSourceManager());
   llvm::StringSet<> ReferencedFileNames;
   auto &SM = AST.getSourceManager();
   for (FileID FID : ReferencedFiles.User)
@@ -493,15 +497,31 @@
   TestTU TU;
   TU.Code = R"cpp(
 #include "behind_keep.h" // IWYU pragma: keep
+#include "public.h"
+
+void bar() { foo(); }
 )cpp";
   TU.AdditionalFiles["behind_keep.h"] = guard("");
+  TU.AdditionalFiles["public.h"] = guard("#include \"private.h\"");
+  TU.AdditionalFiles["private.h"] = guard(R"cpp(
+// IWYU pragma: private, include "public.h"
+void foo() {}
+  )cpp");
   ParsedAST AST = TU.build();
 
-  auto ReferencedFiles =
-  findReferencedFiles(findReferencedLocations(AST),
-  AST.getIncludeStructure(), AST.getSourceManager());
-  EXPECT_TRUE(ReferencedFiles.User.empty());
+  auto ReferencedFiles = findReferencedFiles(
+  findReferencedLocations(AST), AST.getIncludeStructure(),
+  AST.getCanonicalIncludes(), AST.getSourceManager());
+  EXPECT_EQ(ReferencedFiles.SpelledUmbrellas.size(), 1u);
+  EXPECT_EQ(ReferencedFiles.SpelledUmbrellas.begin()->getKey(), "\"public.h\"");
+  EXPECT_EQ(ReferencedFiles.User.size(), 2u);
+  EXPECT_TRUE(
+  ReferencedFiles.User.contains(AST.getSourceManager().getMainFileID()));
+  EXPECT_TRUE(
+  ReferencedFiles.User.contains(AST.getSourceManager().getMainFileID()));
   EXPECT_THAT(AST.getDiagnostics(), llvm::ValueIs(IsEmpty()));
+  auto Unused = computeUnusedIncludes(AST);
+  EXPECT_THAT(Unused, IsEmpty());
 }
 
 } // namespace
Index: clang-tools-extra/clangd/IncludeCleaner.h
===
--- clang-tools-extra/clangd/IncludeCleaner.h
+++ clang-tools-extra/clangd/IncludeCleaner.h
@@ -13,9 +13,6 @@
 /

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

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D122627#3417919 , @beanz wrote:

> In D122627#3417557 , @aaron.ballman 
> wrote:
>
>> Are you sure that's what you want? This returns true for a static C++ member 
>> function, false for a static free function, and false for within an unnamed 
>> namespace, and true otherwise.
>
> You're right this isn't quite right, but getting closer... HLSL doesn't 
> support unnamed namespaces, and we only support static free functions by 
> accident... (the current compiler ignores static on free functions).
>
> This actually revealed some gaps in the documentation for HLSL, I've gone 
> back and gotten feedback from my team's HLSL expert and I think I've got the 
> right set of constraints for where this can be applied now (clang will even 
> have this better than the HLSL compiler).

SGTM, thanks for digging into it!

>> Also, I didn't see any new test coverage for function merging behavior.
>
> Doh! I knew I was forgetting something. Juggling too many balls today. I'll 
> get that covered too!

No worries, thanks for following up on it. :-)


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

[PATCH] D120952: [clang][AST matchers] adding submatchers under cudaKernelCallExpr to match kernel launch config

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

Thanks! This is missing test coverage for the new matcher, and you should also 
add a release note for it.




Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:7837-7839
+  if (const CallExpr *Config = Node.getConfig()) {
+return InnerMatcher.matches(*Config, Finder, Builder);
+  }




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120952

___
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

[Gabor Marton via Phabricator via cfe-commits]

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

Do import the definition of objects from a foreign translation unit if that's 
type is const and trivial.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D122805

Files:
  clang/include/clang/CrossTU/CrossTranslationUnit.h
  clang/lib/CrossTU/CrossTranslationUnit.cpp
  clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
  clang/test/Analysis/Inputs/ctu-other.cpp
  clang/test/Analysis/Inputs/ctu-other.cpp.externalDefMap.ast-dump.txt
  clang/test/Analysis/ctu-main.cpp
  clang/test/Analysis/func-mapping-test.cpp
  clang/tools/clang-extdef-mapping/ClangExtDefMapGen.cpp

Index: clang/tools/clang-extdef-mapping/ClangExtDefMapGen.cpp
===
--- clang/tools/clang-extdef-mapping/ClangExtDefMapGen.cpp
+++ clang/tools/clang-extdef-mapping/ClangExtDefMapGen.cpp
@@ -64,7 +64,7 @@
   if (const Stmt *Body = FD->getBody())
 addIfInMain(FD, Body->getBeginLoc());
   } else if (const auto *VD = dyn_cast(D)) {
-if (cross_tu::containsConst(VD, Ctx) && VD->hasInit())
+if (cross_tu::shouldImport(VD, Ctx) && VD->hasInit())
   if (const Expr *Init = VD->getInit())
 addIfInMain(VD, Init->getBeginLoc());
   }
Index: clang/test/Analysis/func-mapping-test.cpp
===
--- clang/test/Analysis/func-mapping-test.cpp
+++ clang/test/Analysis/func-mapping-test.cpp
@@ -23,7 +23,7 @@
 struct SF {
   const int a;
 };
-SF sf = {.a = 2};
+extern const SF sf = {.a = 2};
 // CHECK-DAG: c:@sf
 
 struct SStatic {
@@ -39,7 +39,7 @@
   const int a;
   const unsigned int b;
 };
-U u = {.a = 6};
+extern const U u = {.a = 6};
 // CHECK-DAG: c:@u
 
 // No USR can be generated for this.
Index: clang/test/Analysis/ctu-main.cpp
===
--- clang/test/Analysis/ctu-main.cpp
+++ clang/test/Analysis/ctu-main.cpp
@@ -75,6 +75,12 @@
   int a;
 };
 extern const S extS;
+struct NonTrivialS {
+  int a;
+  // User declaring a dtor makes it non-trivial.
+  ~NonTrivialS();
+};
+extern const NonTrivialS extNTS;
 extern const int extHere;
 const int extHere = 6;
 struct A {
@@ -83,9 +89,9 @@
 struct SC {
   const int a;
 };
-extern SC extSC;
+extern const SC extSC;
 struct ST {
-  static struct SC sc;
+  static const struct SC sc;
 };
 struct SCNest {
   struct SCN {
@@ -93,7 +99,7 @@
   } scn;
 };
 extern SCNest extSCN;
-extern SCNest::SCN extSubSCN;
+extern const SCNest::SCN extSubSCN;
 struct SCC {
   SCC(int c);
   const int a;
@@ -103,7 +109,7 @@
   const int a;
   const unsigned int b;
 };
-extern U extU;
+extern const U extU;
 
 void test_virtual_functions(mycls* obj) {
   // The dynamic type is known.
@@ -172,6 +178,8 @@
   clang_analyzer_eval(extInt == 2); // expected-warning{{TRUE}}
   clang_analyzer_eval(intns::extInt == 3); // expected-warning{{TRUE}}
   clang_analyzer_eval(extS.a == 4); // expected-warning{{TRUE}}
+  // Do not import non-trivial classes' initializers.
+  clang_analyzer_eval(extNTS.a == 4); // expected-warning{{UNKNOWN}}
   clang_analyzer_eval(extHere == 6); // expected-warning{{TRUE}}
   clang_analyzer_eval(A::a == 3); // expected-warning{{TRUE}}
   clang_analyzer_eval(extSC.a == 8); // expected-warning{{TRUE}}
Index: clang/test/Analysis/Inputs/ctu-other.cpp.externalDefMap.ast-dump.txt
===
--- clang/test/Analysis/Inputs/ctu-other.cpp.externalDefMap.ast-dump.txt
+++ clang/test/Analysis/Inputs/ctu-other.cpp.externalDefMap.ast-dump.txt
@@ -18,6 +18,7 @@
 c:@extInt ctu-other.cpp.ast
 c:@N@intns@extInt ctu-other.cpp.ast
 c:@extS ctu-other.cpp.ast
+c:@extNTS ctu-other.cpp.ast
 c:@S@A@a ctu-other.cpp.ast
 c:@extSC ctu-other.cpp.ast
 c:@S@ST@sc ctu-other.cpp.ast
@@ -27,4 +28,4 @@
 c:@extU ctu-other.cpp.ast
 c:@S@TestAnonUnionUSR@Test ctu-other.cpp.ast
 c:@F@testImportOfIncompleteDefaultParmDuringImport#I# ctu-other.cpp.ast
-c:@F@testImportOfDelegateConstructor#I# ctu-other.cpp.ast
\ No newline at end of file
+c:@F@testImportOfDelegateConstructor#I# ctu-other.cpp.ast
Index: clang/test/Analysis/Inputs/ctu-other.cpp
===
--- clang/test/Analysis/Inputs/ctu-other.cpp
+++ clang/test/Analysis/Inputs/ctu-other.cpp
@@ -102,6 +102,11 @@
   int a;
 };
 extern const S extS = {.a = 4};
+struct NonTrivialS {
+  int a;
+  ~NonTrivialS();
+};
+extern const NonTrivialS extNTS = {.a = 4};
 struct A {
   static const int a;
 };
@@ -109,18 +114,18 @@
 struct SC {
   const int a;
 };
-SC extSC = {.a = 8};
+exter

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

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

steakhal added a comment.

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.




Comment at: clang/include/clang/CrossTU/CrossTranslationUnit.h:112
 
-// Returns true if the variable or any field of a record variable is const.
-bool containsConst(const VarDecl *VD, const ASTContext &ACtx);
+/// Returns true if it makes sense to import a foreign variable declaration.
+/// For instance, we don't want to import variables that have non-trivial types

definition



Comment at: clang/lib/CrossTU/CrossTranslationUnit.cpp:195
+bool shouldImport(const VarDecl *VD, const ASTContext &ACtx) {
+
   CanQualType CT = ACtx.getCanonicalType(VD->getType());

extra blank line



Comment at: clang/test/Analysis/Inputs/ctu-other.cpp:128
 SCNest extSCN = {.scn = {.a = 9}};
-SCNest::SCN extSubSCN = {.a = 1};
+extern SCNest::SCN const extSubSCN = {.a = 1};
 struct SCC {

AFAIK you don't need the `extern` here, since **this is** the definition of 
that global, not only a declaration.
Similarly applies to the rest of the cases.


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] D122732: [Clang][AArch64][SVE] Allow subscript operator for SVE types

[David Truby via Phabricator via cfe-commits]

DavidTruby added a comment.

> Not sure what you mean by this; LLVM supports extractelement on ` x i1>` vectors.  I guess the fact that it's a "vscale x 16" element vector 
> might not be intuitive?

It's a native operation at the LLVM level but not at the ISA level, unlike the 
data registers. Code quality would be quite poor if we just allowed it naively 
so I thought it better to disallow it to not give the impression it's 
easy/free. I will update the text in the commit message to make this more clear.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122732

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

[clang] 7f07600 - [clang][dataflow] Add support for `value_or` in a comparison.

[Yitzhak Mandelbaum via cfe-commits]

Author: Yitzhak Mandelbaum
Date: 2022-03-31T13:21:39Z
New Revision: 7f076004e941fe60ab613a218da31a25b09b0925

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

LOG: [clang][dataflow] Add support for `value_or` in a comparison.

This patch adds limited modeling of the `value_or` method. Specifically, when
used in a particular idiom in a comparison to implicitly check whether the
optional holds a value.

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

Added: 


Modified: 
clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp

Removed: 




diff  --git 
a/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp 
b/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
index b775698dafb5d..aaf8289c48674 100644
--- a/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
+++ b/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
@@ -121,6 +121,43 @@ auto isStdSwapCall() {
   hasArgument(1, hasOptionalType()));
 }
 
+constexpr llvm::StringLiteral ValueOrCallID = "ValueOrCall";
+
+auto isValueOrStringEmptyCall() {
+  // `opt.value_or("").empty()`
+  return cxxMemberCallExpr(
+  callee(cxxMethodDecl(hasName("empty"))),
+  onImplicitObjectArgument(ignoringImplicit(
+  cxxMemberCallExpr(on(expr(unless(cxxThisExpr(,
+callee(cxxMethodDecl(hasName("value_or"),
+ ofClass(optionalClass(,
+hasArgument(0, stringLiteral(hasSize(0
+  .bind(ValueOrCallID;
+}
+
+auto isValueOrNotEqX() {
+  auto ComparesToSame = [](ast_matchers::internal::Matcher Arg) {
+return hasOperands(
+ignoringImplicit(
+cxxMemberCallExpr(on(expr(unless(cxxThisExpr(,
+  callee(cxxMethodDecl(hasName("value_or"),
+   ofClass(optionalClass(,
+  hasArgument(0, Arg))
+.bind(ValueOrCallID)),
+ignoringImplicit(Arg));
+  };
+
+  // `opt.value_or(X) != X`, for X is `nullptr`, `""`, or `0`. Ideally, we'd
+  // support this pattern for any expression, but the AST does not have a
+  // generic expression comparison facility, so we specialize to common cases
+  // seen in practice.  FIXME: define a matcher that compares values across
+  // nodes, which would let us generalize this to any `X`.
+  return binaryOperation(hasOperatorName("!="),
+ anyOf(ComparesToSame(cxxNullPtrLiteralExpr()),
+   ComparesToSame(stringLiteral(hasSize(0))),
+   ComparesToSame(integerLiteral(equals(0);
+}
+
 /// Creates a symbolic value for an `optional` value using `HasValueVal` as the
 /// symbolic value of its "has_value" property.
 StructValue &createOptionalValue(Environment &Env, BoolValue &HasValueVal) {
@@ -220,6 +257,69 @@ void transferOptionalHasValueCall(const CXXMemberCallExpr 
*CallExpr,
   }
 }
 
+/// `ModelPred` builds a logical formula relating the predicate in
+/// `ValueOrPredExpr` to the optional's `has_value` property.
+void transferValueOrImpl(const clang::Expr *ValueOrPredExpr,
+ const MatchFinder::MatchResult &Result,
+ LatticeTransferState &State,
+ BoolValue &(*ModelPred)(Environment &Env,
+ BoolValue &ExprVal,
+ BoolValue &HasValueVal)) {
+  auto &Env = State.Env;
+
+  const auto *ObjectArgumentExpr =
+  Result.Nodes.getNodeAs(ValueOrCallID)
+  ->getImplicitObjectArgument();
+
+  auto *OptionalVal = cast_or_null(
+  Env.getValue(*ObjectArgumentExpr, SkipPast::ReferenceThenPointer));
+  if (OptionalVal == nullptr)
+return;
+  auto *HasValueVal = getHasValue(OptionalVal);
+  assert(HasValueVal != nullptr);
+
+  auto *ExprValue = cast_or_null(
+  State.Env.getValue(*ValueOrPredExpr, SkipPast::None));
+  if (ExprValue == nullptr) {
+auto &ExprLoc = State.Env.createStorageLocation(*ValueOrPredExpr);
+ExprValue = &State.Env.makeAtomicBoolValue();
+State.Env.setValue(ExprLoc, *ExprValue);
+State.Env.setStorageLocation(*ValueOrPredExpr, ExprLoc);
+  }
+
+  Env.addToFlowCondition(ModelPred(Env, *ExprValue, *HasValueVal));
+}
+
+void transferValueOrStringEmptyCall(const clang::Expr *ComparisonExpr,
+const MatchFinder::MatchResult &Result,
+LatticeTransferState &State) {
+  return transferValueOrIm

[PATCH] D122231: [clang][dataflow] Add support for `value_or` in a comparison.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG7f076004e941: [clang][dataflow] Add support for `value_or` 
in a comparison. (authored by ymandel).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122231

Files:
  clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
  clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp

Index: clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
@@ -496,6 +496,24 @@
 #endif // ABSL_TYPE_TRAITS_H
 )";
 
+static constexpr char StdStringHeader[] = R"(
+#ifndef STRING_H
+#define STRING_H
+
+namespace std {
+
+struct string {
+  string(const char*);
+  ~string();
+  bool empty();
+};
+bool operator!=(const string &LHS, const char *RHS);
+
+} // namespace std
+
+#endif // STRING_H
+)";
+
 static constexpr char StdUtilityHeader[] = R"(
 #ifndef UTILITY_H
 #define UTILITY_H
@@ -1198,6 +1216,7 @@
 std::vector> Headers;
 Headers.emplace_back("cstddef.h", CSDtdDefHeader);
 Headers.emplace_back("std_initializer_list.h", StdInitializerListHeader);
+Headers.emplace_back("std_string.h", StdStringHeader);
 Headers.emplace_back("std_type_traits.h", StdTypeTraitsHeader);
 Headers.emplace_back("std_utility.h", StdUtilityHeader);
 Headers.emplace_back("std_optional.h", StdOptionalHeader);
@@ -1209,6 +1228,7 @@
   #include "base_optional.h"
   #include "std_initializer_list.h"
   #include "std_optional.h"
+  #include "std_string.h"
   #include "std_utility.h"
 
   template 
@@ -1712,6 +1732,102 @@
  UnorderedElementsAre(Pair("check", "safe")));
 }
 
+TEST_P(UncheckedOptionalAccessTest, ValueOrComparison) {
+  // Pointers.
+  ExpectLatticeChecksFor(
+  R"code(
+#include "unchecked_optional_access_test.h"
+
+void target($ns::$optional opt) {
+  if (opt.value_or(nullptr) != nullptr) {
+opt.value();
+/*[[check-ptrs-1]]*/
+  } else {
+opt.value();
+/*[[check-ptrs-2]]*/
+  }
+}
+  )code",
+  UnorderedElementsAre(Pair("check-ptrs-1", "safe"),
+   Pair("check-ptrs-2", "unsafe: input.cc:9:9")));
+
+  // Integers.
+  ExpectLatticeChecksFor(
+  R"code(
+#include "unchecked_optional_access_test.h"
+
+void target($ns::$optional opt) {
+  if (opt.value_or(0) != 0) {
+opt.value();
+/*[[check-ints-1]]*/
+  } else {
+opt.value();
+/*[[check-ints-2]]*/
+  }
+}
+  )code",
+  UnorderedElementsAre(Pair("check-ints-1", "safe"),
+   Pair("check-ints-2", "unsafe: input.cc:9:9")));
+
+  // Strings.
+  ExpectLatticeChecksFor(
+  R"code(
+#include "unchecked_optional_access_test.h"
+
+void target($ns::$optional opt) {
+  if (!opt.value_or("").empty()) {
+opt.value();
+/*[[check-strings-1]]*/
+  } else {
+opt.value();
+/*[[check-strings-2]]*/
+  }
+}
+  )code",
+  UnorderedElementsAre(Pair("check-strings-1", "safe"),
+   Pair("check-strings-2", "unsafe: input.cc:9:9")));
+
+  ExpectLatticeChecksFor(
+  R"code(
+#include "unchecked_optional_access_test.h"
+
+void target($ns::$optional opt) {
+  if (opt.value_or("") != "") {
+opt.value();
+/*[[check-strings-neq-1]]*/
+  } else {
+opt.value();
+/*[[check-strings-neq-2]]*/
+  }
+}
+  )code",
+  UnorderedElementsAre(
+  Pair("check-strings-neq-1", "safe"),
+  Pair("check-strings-neq-2", "unsafe: input.cc:9:9")));
+
+  // Pointer-to-optional.
+  //
+  // FIXME: make `opt` a parameter directly, once we ensure that all `optional`
+  // values have a `has_value` property.
+  ExpectLatticeChecksFor(
+  R"code(
+#include "unchecked_optional_access_test.h"
+
+void target($ns::$optional p) {
+  $ns::$optional *opt = &p;
+  if (opt->value_or(0) != 0) {
+opt->value();
+/*[[check-pto-1]]*/
+  } else {
+opt->value();
+/*[[check-pto-2]]*/
+  }
+}
+  )code",
+  UnorderedElementsAre(Pair("check-pto-1", "safe"),
+   Pair("check-pto-2", "unsafe: input.cc:10:9")));
+}
+
 TEST_P(UncheckedOptionalAccessTest, Emplace) {
   ExpectLatticeChecksFor(R"(
 #include "unchecked_optional_access_test.h"
@@ -2038,5 +2154,4 @@
 // - constructors (copy, move)
 // - assignment operators (default, copy, move)
 // - invalidation (passing optional by non-const reference/pointer)
-// - `value_or(nullptr) != nullptr`, `value_or(0) != 0`, `value_or("").empty()`
 // - nested `optional` values
In

[PATCH] D122808: [clang] Fix warnings with `-Wdeprecated-enum-enum-conversion` is enabled

[Antonio Frighetto via Phabricator via cfe-commits]

antoniofrighetto created this revision.
antoniofrighetto added reviewers: aaron.ballman, dblaikie.
antoniofrighetto added a project: clang.
Herald added a project: All.
antoniofrighetto requested review of this revision.
Herald added a subscriber: cfe-commits.

clang may throw the following warning: 
`include/clang/AST/DeclarationName.h:210:52: error: arithmetic between 
different enumeration types ('clang::DeclarationName::StoredNameKind' and 
'clang::detail::DeclarationNameExtra::ExtraKind') is deprecated` when flags 
`-Werror,-Wdeprecated-enum-enum-conversion` are on.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D122808

Files:
  clang/include/clang/AST/DeclarationName.h


Index: clang/include/clang/AST/DeclarationName.h
===
--- clang/include/clang/AST/DeclarationName.h
+++ clang/include/clang/AST/DeclarationName.h
@@ -27,6 +27,7 @@
 #include 
 #include 
 #include 
+#include 
 
 namespace clang {
 
@@ -192,6 +193,13 @@
 "The various classes that DeclarationName::Ptr can point to"
 " must be at least aligned to 8 bytes!");
 
+  static_assert(
+  std::is_same,
+   std::underlying_type_t<
+   detail::DeclarationNameExtra::ExtraKind>>::value,
+  "The various enums used to compute values for NameKind should all have "
+  "the same underlying type");
+
 public:
   /// The kind of the name stored in this DeclarationName.
   /// The first 7 enumeration values are stored inline and correspond
@@ -205,15 +213,30 @@
 CXXDestructorName = StoredCXXDestructorName,
 CXXConversionFunctionName = StoredCXXConversionFunctionName,
 CXXOperatorName = StoredCXXOperatorName,
-CXXDeductionGuideName = UncommonNameKindOffset +
-
detail::DeclarationNameExtra::CXXDeductionGuideName,
+CXXDeductionGuideName =
+static_cast>(
+UncommonNameKindOffset) +
+static_cast<
+std::underlying_type_t>(
+detail::DeclarationNameExtra::CXXDeductionGuideName),
 CXXLiteralOperatorName =
-UncommonNameKindOffset +
-detail::DeclarationNameExtra::CXXLiteralOperatorName,
-CXXUsingDirective = UncommonNameKindOffset +
-detail::DeclarationNameExtra::CXXUsingDirective,
-ObjCMultiArgSelector = UncommonNameKindOffset +
-   detail::DeclarationNameExtra::ObjCMultiArgSelector
+static_cast>(
+UncommonNameKindOffset) +
+static_cast<
+std::underlying_type_t>(
+detail::DeclarationNameExtra::CXXLiteralOperatorName),
+CXXUsingDirective =
+static_cast>(
+UncommonNameKindOffset) +
+static_cast<
+std::underlying_type_t>(
+detail::DeclarationNameExtra::CXXUsingDirective),
+ObjCMultiArgSelector =
+static_cast>(
+UncommonNameKindOffset) +
+static_cast<
+std::underlying_type_t>(
+detail::DeclarationNameExtra::ObjCMultiArgSelector),
   };
 
 private:


Index: clang/include/clang/AST/DeclarationName.h
===
--- clang/include/clang/AST/DeclarationName.h
+++ clang/include/clang/AST/DeclarationName.h
@@ -27,6 +27,7 @@
 #include 
 #include 
 #include 
+#include 
 
 namespace clang {
 
@@ -192,6 +193,13 @@
 "The various classes that DeclarationName::Ptr can point to"
 " must be at least aligned to 8 bytes!");
 
+  static_assert(
+  std::is_same,
+   std::underlying_type_t<
+   detail::DeclarationNameExtra::ExtraKind>>::value,
+  "The various enums used to compute values for NameKind should all have "
+  "the same underlying type");
+
 public:
   /// The kind of the name stored in this DeclarationName.
   /// The first 7 enumeration values are stored inline and correspond
@@ -205,15 +213,30 @@
 CXXDestructorName = StoredCXXDestructorName,
 CXXConversionFunctionName = StoredCXXConversionFunctionName,
 CXXOperatorName = StoredCXXOperatorName,
-CXXDeductionGuideName = UncommonNameKindOffset +
-detail::DeclarationNameExtra::CXXDeductionGuideName,
+CXXDeductionGuideName =
+static_cast>(
+UncommonNameKindOffset) +
+static_cast<
+std::underlying_type_t>(
+detail::DeclarationNameExtra::CXXDeductionGuideName),
 CXXLiteralOperatorName =
-UncommonNameKindOffset +
-detail::DeclarationNameExtra::CXXLiteralOperatorName,
-CXXUsingDirective = UncommonNameKindOffset +
-detail::DeclarationNameExtra::CXXUsingDirective,
-ObjCMultiArgSelector = UncommonNameKindOffset +
-   detail::DeclarationNameExtra::ObjCMultiArgSelector
+static_cast>(
+UncommonNameKindOffset) +
+stati

[PATCH] D121556: [randstruct] Add randomize structure layout support

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

Generally I think things are looking pretty good, but there's still an open 
question and Precommit CI is still failing on Windows:

  Failed Tests (7):
Clang-Unit :: 
AST/./ASTTests.exe/StructureLayoutRandomization.AnonymousStructsAndUnionsRetainFieldOrder
Clang-Unit :: 
AST/./ASTTests.exe/StructureLayoutRandomization.CheckAdjacentBitfieldsRemainAdjacentAfterRandomization
Clang-Unit :: 
AST/./ASTTests.exe/StructureLayoutRandomization.CheckVariableLengthArrayMemberRemainsAtEndOfStructure
Clang-Unit :: 
AST/./ASTTests.exe/StructureLayoutRandomization.MarkedRandomize
Clang-Unit :: 
AST/./ASTTests.exe/StructureLayoutRandomization.MultipleAttrsRandomize
Clang-Unit :: 
AST/./ASTTests.exe/StructureLayoutRandomization.RandstructDoesNotOverrideThePackedAttr
Clang-Unit :: 
AST/./ASTTests.exe/StructureLayoutRandomization.ZeroWidthBitfieldsSeparateAllocationUnits




Comment at: clang/include/clang/Basic/Attr.td:3954-3968
+def RandomizeLayout : InheritableAttr {
+  let Spellings = [GCC<"randomize_layout">];
+  let Subjects = SubjectList<[Record]>;
+  let Documentation = [ClangRandstructDocs];
+  let LangOpts = [COnly];
+}
+

This gets rid of one of the AST nodes and uses an accessor instead. e.g., the 
user can still write `[[gnu::no_randomize_layout]]` on a struct declaration, 
we'll generate a `RandomizeLayoutAttr` for it, and you can call 
`->isDisabled()` on an object to tell whether it's the `no_` spelling or not.

I don't feel super strong about this change, so if it turns out to make code 
elsewhere significantly worse, feel free to ignore. My reasoning for combing is 
that the `no` variant doesn't carry much semantic weight but eats up an 
attribute kind value just the same; it seemed like a heavy use for an AST node.



Comment at: clang/lib/Sema/SemaDeclAttr.cpp:8550-8556
+  case ParsedAttr::AT_RandomizeLayout:
+handleSimpleAttribute(S, D, AL);
+break;
+  case ParsedAttr::AT_NoRandomizeLayout:
+// Drop the "randomize_layout" attribute if it's on the decl.
+D->dropAttr();
+break;

void wrote:
> aaron.ballman wrote:
> > I don't think this is sufficient. Consider redeclaration merging cases:
> > ```
> > struct [[gnu::randomize_layout]] S;
> > struct [[gnu::no_randomize_layout]] S {};
> > 
> > struct [[gnu::no_randomize_layout]] T;
> > struct [[gnu::randomize_layout]] T {};
> > ```
> > I think if the user accidentally does something like this, it should be 
> > diagnosed. I would have assumed this would warrant an error diagnostic 
> > because the user is obviously confused as to what they want, but it seems 
> > GCC ignores the subsequent diagnostic with a warning: 
> > https://godbolt.org/z/1q8dazYPW.
> > 
> > There's also the "confused attribute list" case which GCC just... does... 
> > things... with: https://godbolt.org/z/rnfsn7hG1. I think we want to behave 
> > more consistently with how we treat these cases.
> > 
> > Either way, we shouldn't be silent.
> The GCC feature is done via a plugin in Linux. So godbolt probably won't work 
> in this case. I'll check to see how GCC responds to these attribute 
> situations.
Thoughts on this?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121556

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

[libunwind] cb055e5 - [libc++] Add a CI job running MSAN

[Louis Dionne via cfe-commits]

Author: Louis Dionne
Date: 2022-03-31T09:31:22-04:00
New Revision: cb055e51f994806160465212ac100485dac48125

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

LOG: [libc++] Add a CI job running MSAN

For some reason, we've been going without a MSAN CI job, even though
even run-buildbot defined a generic-msan job. This must have been an
oversight that went unnoticed. Thanks to @EricWF for the catch.

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

Added: 


Modified: 
libcxx/utils/ci/buildkite-pipeline.yml
libunwind/test/forceunwind.pass.cpp
libunwind/test/libunwind_01.pass.cpp
libunwind/test/libunwind_02.pass.cpp
libunwind/test/remember_state_leak.pass.sh.s
libunwind/test/signal_frame.pass.cpp
libunwind/test/signal_unwind.pass.cpp
libunwind/test/unwind_leaffunction.pass.cpp

Removed: 




diff  --git a/libcxx/utils/ci/buildkite-pipeline.yml 
b/libcxx/utils/ci/buildkite-pipeline.yml
index 7653ead09ae6c..5a6f36bea3c98 100644
--- a/libcxx/utils/ci/buildkite-pipeline.yml
+++ b/libcxx/utils/ci/buildkite-pipeline.yml
@@ -276,6 +276,20 @@ steps:
 limit: 2
   timeout_in_minutes: 120
 
+- label: "MSAN"
+  command: "libcxx/utils/ci/run-buildbot generic-msan"
+  artifact_paths:
+- "**/test-results.xml"
+- "**/*.abilist"
+  agents:
+queue: "libcxx-builders"
+os: "linux"
+  retry:
+automatic:
+  - exit_status: -1  # Agent was lost
+limit: 2
+  timeout_in_minutes: 120
+
   # Tests with the various supported ways to build libc++.
   - label: "Bootstrapping build"
 command: "libcxx/utils/ci/run-buildbot bootstrapping-build"

diff  --git a/libunwind/test/forceunwind.pass.cpp 
b/libunwind/test/forceunwind.pass.cpp
index 466697264035b..af5f234b8da68 100644
--- a/libunwind/test/forceunwind.pass.cpp
+++ b/libunwind/test/forceunwind.pass.cpp
@@ -9,6 +9,9 @@
 
 // REQUIRES: linux
 
+// TODO: Figure out why this fails with Memory Sanitizer.
+// XFAIL: msan
+
 // Basic test for _Unwind_ForcedUnwind.
 // See libcxxabi/test/forced_unwind* tests too.
 

diff  --git a/libunwind/test/libunwind_01.pass.cpp 
b/libunwind/test/libunwind_01.pass.cpp
index e5737450a568f..d89f8d16ce70d 100644
--- a/libunwind/test/libunwind_01.pass.cpp
+++ b/libunwind/test/libunwind_01.pass.cpp
@@ -1,6 +1,9 @@
 // TODO: Investigate these failures on x86_64 macOS back deployment
 // UNSUPPORTED: target=x86_64-apple-darwin{{.+}}
 
+// TODO: Figure out why this fails with Memory Sanitizer.
+// XFAIL: msan
+
 #include 
 #include 
 #include 

diff  --git a/libunwind/test/libunwind_02.pass.cpp 
b/libunwind/test/libunwind_02.pass.cpp
index b188fad8ee5be..a4f47c521858a 100644
--- a/libunwind/test/libunwind_02.pass.cpp
+++ b/libunwind/test/libunwind_02.pass.cpp
@@ -1,3 +1,6 @@
+// TODO: Figure out why this fails with Memory Sanitizer.
+// XFAIL: msan
+
 #include 
 #include 
 #include 

diff  --git a/libunwind/test/remember_state_leak.pass.sh.s 
b/libunwind/test/remember_state_leak.pass.sh.s
index 590653e2b10de..a02c8213c669c 100644
--- a/libunwind/test/remember_state_leak.pass.sh.s
+++ b/libunwind/test/remember_state_leak.pass.sh.s
@@ -1,4 +1,8 @@
 # REQUIRES: target={{x86_64-.+-linux-gnu}}
+
+// Inline assembly isn't supported by Memory Sanitizer
+// UNSUPPORTED: msan
+
 # RUN: %{build} -no-pie
 # RUN: %{run}
 

diff  --git a/libunwind/test/signal_frame.pass.cpp 
b/libunwind/test/signal_frame.pass.cpp
index 85a883be4e5fe..d9fb439cd1e7f 100644
--- a/libunwind/test/signal_frame.pass.cpp
+++ b/libunwind/test/signal_frame.pass.cpp
@@ -12,6 +12,9 @@
 // TODO: Investigate this failure on macOS
 // XFAIL: target={{.+}}-apple-darwin{{.+}}
 
+// TODO: Figure out why this fails with Memory Sanitizer.
+// XFAIL: msan
+
 // UNSUPPORTED: libunwind-arm-ehabi
 
 // The AIX assembler does not support CFI directives, which

diff  --git a/libunwind/test/signal_unwind.pass.cpp 
b/libunwind/test/signal_unwind.pass.cpp
index c16adeb4d18cc..4f2e925349604 100644
--- a/libunwind/test/signal_unwind.pass.cpp
+++ b/libunwind/test/signal_unwind.pass.cpp
@@ -10,6 +10,9 @@
 // Ensure that the unwinder can cope with the signal handler.
 // REQUIRES: linux && (target={{aarch64-.+}} || target={{x86_64-.+}})
 
+// TODO: Figure out why this fails with Memory Sanitizer.
+// XFAIL: msan
+
 #include 
 #include 
 #include 

diff  --git a/libunwind/test/unwind_leaffunction.pass.cpp 
b/libunwind/test/unwind_leaffunction.pass.cpp
index 8ff21dd35449c..a112d755dfeed 100644
--- a/libunwind/test/unwind_leaffunction.pass.cpp
+++ b/libunwind/test/unwind_leaffunction.pass.cpp
@@ -10,6 +10,9 @@
 // Ensure that leaf function can be unwund.
 // REQUIRES: linux && (target={{aarch64-.+}} || target={{x86_64-.+}})
 
+// TODO: Figure out why this fails with 

[PATCH] D120254: [OpenCL] Align subgroup builtin guards

[Harald van Dijk via Phabricator via cfe-commits]

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

In D120254#3342551 , @dyung wrote:

> Hi, our internal release build bots are showing failures in two clang-tidy 
> tests that I bisected back to your commit, 
> clang-tidy/checkers/altera-id-dependent-backward-branch.cpp and 
> clang-tidy/checkers/altera-single-work-item-barrier.cpp. After this change, 
> both are exhibiting this error:
>
>   Error while processing 
> /home/dyung/src/upstream/aa9c2d19d9b73589d72114d6e0a4fb4ce42b922b-linux/tools/clang/tools/extra/test/clang-tidy/checkers/Output/altera-single-work-item-barrier.cpp.tmp.cpp.
>   error: enum type memory_scope not found; include the base header with 
> -finclude-default-header [clang-diagnostic-error]
>
> Oddly, this only fails in a release configuration. Can you take a look?

This was worked around by modifying tests, but I believe this is a fundamental 
problem in this change and was able to reproduce the error with plain old clang:

  $ cat test.cl
  void sub_group_barrier();
  
  $ bin/clang -cl-std=CL1.2 -S -o - test.cl
  error: enum type memory_scope not found; include the base header with 
-finclude-default-header
  1 error generated.
  
  $ bin/clang --version
  clang version 15.0.0 (g...@github.com:llvm/llvm-project 
c204cee642ee794901d2e8a9819b52ac12f92bc9)
  Target: x86_64-unknown-linux-gnu
  Thread model: posix
  InstalledDir: /home/harald/llvm-project/build/bin

The problem is that this change enables certain built-ins in OpenCL 1.2 that 
take a memory_scope argument, but the memory_scope type is not defined in 
OpenCL 1.2 mode. When we then process the function, sub_group_barrier in my 
example, things break when checking whether the declaration matches the 
built-in. I am not sure what the right fix here is. Can we just define the type 
if any extension is enabled that requires the type, or is that not allowed?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120254

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

[PATCH] D118050: [analyzer] Different address spaces cannot overlap

[Vince Bridgers via Phabricator via cfe-commits]

vabridgers updated this revision to Diff 419438.
vabridgers added a comment.

add NDEBUG around assert


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118050

Files:
  clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
  clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
  clang/test/Analysis/cstring-addrspace.c

Index: clang/test/Analysis/cstring-addrspace.c
===
--- /dev/null
+++ clang/test/Analysis/cstring-addrspace.c
@@ -0,0 +1,64 @@
+// RUN: %clang_analyze_cc1 -triple amdgcn-unknown-unknown \
+// RUN: -analyze -analyzer-checker=core,alpha.unix.cstring \
+// RUN: -analyze -analyzer-checker=debug.ExprInspection \
+// RUN: -analyzer-config crosscheck-with-z3=true -verify %s \
+// RUN: -Wno-incompatible-library-redeclaration
+// REQUIRES: z3
+
+void clang_analyzer_warnIfReached();
+
+// From https://llvm.org/docs/AMDGPUUsage.html#address-spaces,
+// select address space 3 (local), since the pointer size is
+// different than Generic.
+#define DEVICE __attribute__((address_space(3)))
+_Static_assert(sizeof(int *) == 8, "");
+_Static_assert(sizeof(DEVICE int *) == 4, "");
+_Static_assert(sizeof(void *) == 8, "");
+_Static_assert(sizeof(DEVICE void *) == 4, "");
+
+// Copy from host to device memory. Note this is specialized
+// since one of the parameters is assigned an address space such
+// that the sizeof the the pointer is different than the other.
+//
+// Some downstream implementations may have specialized memcpy
+// routines that copy from one address space to another. In cases
+// like that, the address spaces are assumed to not overlap, so the
+// cstring overlap check is not needed. When a static analysis report
+// is generated in as case like this, SMTConv may attempt to create
+// a refutation to Z3 with different bitwidth pointers which lead to
+// a crash. This is not common in directly used upstream compiler builds,
+// but can be seen in specialized downstrean implementations. This case
+// covers those specific instances found and debugged.
+//
+// Since memcpy is a builtin, a specialized builtin instance named like
+// 'memcpy_special' will hit in cstring, triggering this behavior. The
+// best we can do for an upstream test is use the same memcpy function name.
+DEVICE void *memcpy(DEVICE void *dst, const void *src, unsigned long len);
+
+void top1(DEVICE void *dst, void *src, int len) {
+  memcpy(dst, src, len);
+
+  // Create a bugreport for triggering Z3 refutation.
+  clang_analyzer_warnIfReached(); // expected-warning {{REACHABLE}}
+}
+
+void top2(DEVICE int *dst, void *src, int len) {
+  memcpy(dst, src, len);
+
+  // Create a bugreport for triggering Z3 refutation.
+  clang_analyzer_warnIfReached(); // expected-warning {{REACHABLE}}
+}
+
+void top3(DEVICE int *dst, int *src, int len) {
+  memcpy(dst, src, len);
+
+  // Create a bugreport for triggering Z3 refutation.
+  clang_analyzer_warnIfReached(); // expected-warning {{REACHABLE}}
+}
+
+void top4() {
+  memcpy((DEVICE void *)1, (const void *)1, 1);
+
+  // Create a bugreport for triggering Z3 refutation.
+  clang_analyzer_warnIfReached(); // expected-warning {{REACHABLE}}
+}
Index: clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
===
--- clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
+++ clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
@@ -715,11 +715,47 @@
   llvm_unreachable("Fields not found in parent record's definition");
 }
 
+#ifndef NDEBUG
+// This is used in debug builds only for now because some downstream users
+// may hit this assert in their subsequent merges.
+// There are still places in the analyzer where equal bitwidth Locs
+// are compared, and need to be found and corrected. Recent previous fixes have
+// addressed the known problems of making NULLs with specific bitwidths
+// for Loc comparisons along with deprecation of APIs for the same purpose.
+//
+static bool assertEqualBitWidths(ProgramStateRef State, Loc RhsLoc,
+ Loc LhsLoc) {
+  // Implements a "best effort" check for RhsLoc and LhsLoc bit widths
+  ASTContext &Ctx = State->getStateManager().getContext();
+  uint64_t RhsBitwidth =
+  RhsLoc.getType(Ctx).isNull() ? 0 : Ctx.getTypeSize(RhsLoc.getType(Ctx));
+  uint64_t LhsBitwidth =
+  LhsLoc.getType(Ctx).isNull() ? 0 : Ctx.getTypeSize(LhsLoc.getType(Ctx));
+  if (RhsBitwidth && LhsBitwidth &&
+  (LhsLoc.getSubKind() == RhsLoc.getSubKind())) {
+assert(RhsBitwidth == LhsBitwidth &&
+   "RhsLoc and LhsLoc bitwidth must be same!");
+return RhsBitwidth == LhsBitwidth;
+  }
+  return false;
+}
+#endif
+
 // FIXME: all this logic will change if/when we have MemRegion::getLocation().
 SVal SimpleSValBuilder::evalBinOpLL(ProgramStateRef state,
   BinaryOperator::Opcode op,
  

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

[Hans Wennborg via Phabricator via cfe-commits]

hans added a comment.

> we should have a flag that controls which slash direction to use on windows 
> triples, since different people will want different things.
> And since most people who use clang-cl run it on Windows, the default for 
> that flag should imho stay a backslash (but projects can add the forward 
> direction flag if they want).

I guess this is the part that's not entirely clear to me. Do people really 
*want* the backslash? Would anything break if we just use the forward slash?

This is MSVC's current behaviour:

  C:\src\tmp>type a.cc foo\a.h
  
  a.cc
  
  
  #include "foo/a.h"
  
  int main() { f(); return 0; }
  
  foo\a.h
  
  
  #include 
  
  inline void f() { printf("%s\n", __FILE__); }
  
  C:\src\tmp>cl /nologo a.cc && a.exe
  a.cc
  C:\src\tmp\foo/a.h

I think

  C:\src\tmp\foo/a.h

looks pretty weird, and canonicalizing on slashes would be an improvement even 
if it diverges from MSVC.

> (I don't think the argument about '\' in include lines applies to this patch 
> (?))

It kinda does in that developers, even on Windows, already use forward slashes 
to refer to included files:

  #include "foo/bar.h"

and so having the compiler prefer forward slashes for the whole filename is in 
line with that.

While this would make clang-cl diverge from MSVC, maybe this is one of those 
cases where we want to diverge because clang-cl's way is better. Unless it 
breaks something.




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

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?


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] D118050: [analyzer] Different address spaces cannot overlap

[Vince Bridgers via Phabricator via cfe-commits]

vabridgers added a comment.

I added #ifndef NDEBUG around the assert since I'm not sure if downstream 
consumers that use different pointer sizes by address space will hit this 
assert. Our downstream target causes this assert to fire, so I will be 
continuing to find and fix these issues, submitting test cases as I can. Heck, 
we may even have to submit a mock target to properly suss these out :)

Thanks for the support for sorting these out. Best


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118050

___
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

[Hans Wennborg via Phabricator via cfe-commits]

hans added reviewers: mstorsjo, aaron.ballman.
hans added a comment.

Maybe Martin or Aaron have opinions here too?


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] D122698: [clangd] Add support to extract method for ExtractFunction Tweak

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

OK, I think the main remaining thing is enclosing contexts... (sorry, I wasn't 
thinking clearly about these in the last round, and I think existing code gets 
this slightly wrong too).

Each declaration has potentially *two* contexts, semantic and syntactic, and 
the distinction is relevant here.

  namespace ns {
  class A {
void f(); // SemanticDC = A, SyntacticDC = A
  };
  void A::f() { } // SemanticDC = A, SyntacticDC = ns
  }

To determine how to spell different parts of the function, different rules 
apply to different parts of the declaration:

- return type and function name needs to be relative to the SyntacticDC
- parameters and function body are relative to the SemanticDC

I think `renderDeclaration()` should take SemanticDC and SyntacticDC as 
parameters, and NewFunction should have SemanticDC, SyntacticDC, 
ForwardDeclarationSyntacticDC as fields. (By definition, the forward 
declaration and the definition have the same semantic DC).

To summarize, something like:

  struct NewFunction {
DeclContext *SemanticDC;
DeclContext *SyntacticDC;
DeclContext *ForwardDeclarationSyntacticDC;
  }
  void renderDeclaration(FunctionDeclKind, DeclContext& SemanticDC, DeclContext 
&SyntacticDC, ...) {
... printType(ReturnType, SyntacticDC), ..., 
renderParametersForDefinition(SemanticDC)...
  }
  void createFunctionDefinition(...) {
 renderDeclaration(..., SemanticDC, SyntacticDC, ...);
  }
  void createForwardDeclaration(...) {
renderDeclaration(..., SemanticDC, ForwardDeclarationSyntacticDC, ...);
  }




Comment at: clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp:352
+  SourceLocation DefinitionPoint;
+  llvm::Optional ForwardDeclarationPoint;
+  const CXXRecordDecl *EnclosingClass = nullptr;

you only ever use the beginning of this range (and it's an insertion point, so 
having a range doesn't make much sense). Change to SourceLocation?



Comment at: clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp:354
+  const CXXRecordDecl *EnclosingClass = nullptr;
+  NestedNameSpecifier *NestedNameSpec = nullptr;
+  const DeclContext *EnclosingFuncContext = nullptr;

nit: const (the AST isn't great about const-correctness, but should be OK here)

Maybe NestedNameSpec -> DefinitionQualifier?



Comment at: clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp:361
+
+  const DeclContext &getEnclosing() const;
+

this needs a more specific name - see overall comment on the patch



Comment at: clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp:366
   tooling::ExtractionSemicolonPolicy SemicolonPolicy;
-  NewFunction(tooling::ExtractionSemicolonPolicy SemicolonPolicy)
-  : SemicolonPolicy(SemicolonPolicy) {}
+  const LangOptions &LangOpts;
+  NewFunction(tooling::ExtractionSemicolonPolicy SemicolonPolicy,

nit: pointer rather than reference, reference members have surprising effects 
and we don't have any here so far



Comment at: clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp:473
 
-std::string NewFunction::renderDefinition(const SourceManager &SM) const {
+std::string NewFunction::renderDefinition(FunctionDeclKind K,
+  const DeclContext &Enclosing,

you don't need separate renderDeclaration() and renderDefinition() functions: 
the FunctionDeclKind tells you whether this is a definition or not.

(And naming-wise, definitions are just a special type of declaration)



Comment at: clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp:847
+
+tooling::Replacement createFunctionDeclaration(const NewFunction 
&ExtractedFunc,
+   const SourceManager &SM) {

nit: createForwardDeclaration



Comment at: clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp:455
+  return std::string(llvm::formatv("{0}{1} {2}({3}){4}", renderAttributes(),
+   printType(ReturnType, 
*EnclosingFuncContext),
+   Name, renderParametersForDefinition(),

FabioRS wrote:
> sammccall wrote:
> > For out-of-line defined methods, this is the wrong DC, which may lead to 
> > the type being incorrectly qualified. The DC should rather be the enclosing 
> > class.
> > 
> > Similarly, renderParametersForDefinition uses EnclosingFuncContext too - 
> > instead it should take the EnclosingFuncContext as a parameter. (And be 
> > renamed `renderParametersForDeclaration`, since it's no longer just for 
> > definitions)
> It is only for out-of-line methods? I put this verification in the 
> getEnclosing() of the new diff.
> 
functions can also be defined in a different syntactic context than they are 
declared:
```
namespace ns { void foo(); }
void ns::foo() {}
```

This is 

[PATCH] D120254: [OpenCL] Align subgroup builtin guards

[Sven van Haastregt via Phabricator via cfe-commits]

svenvh added a comment.

In D120254#3419221 , @hvdijk wrote:

> This was worked around by modifying tests, but I believe this is a 
> fundamental problem in this change and was able to reproduce the error with 
> plain old clang:
>
>   $ cat test.cl
>   void sub_group_barrier();
>   
>   $ bin/clang -cl-std=CL1.2 -S -o - test.cl
>   error: enum type memory_scope not found; include the base header with 
> -finclude-default-header
>   1 error generated.
>   
>   $ bin/clang --version
>   clang version 15.0.0 (g...@github.com:llvm/llvm-project 
> c204cee642ee794901d2e8a9819b52ac12f92bc9)
>   Target: x86_64-unknown-linux-gnu
>   Thread model: posix
>   InstalledDir: /home/harald/llvm-project/build/bin
>
> The problem is that this change enables certain built-ins in OpenCL 1.2 that 
> take a memory_scope argument, but the memory_scope type is not defined in 
> OpenCL 1.2 mode. When we then process the function, sub_group_barrier in my 
> example, things break when checking whether the declaration matches the 
> built-in. I am not sure what the right fix here is. Can we just define the 
> type if any extension is enabled that requires the type, or is that not 
> allowed?

Thanks for digging further and providing a reproducer!  I think the fix is to 
only make the `sub_group_barrier(cl_mem_fence_flags flags, memory_scope)` 
overload available for OpenCL 2.0 or above.  That would also match `opencl-c.h`.

The following patch seems to fix the issue that you described:

  diff --git a/clang/lib/Sema/OpenCLBuiltins.td 
b/clang/lib/Sema/OpenCLBuiltins.td
  index f6de59223347..52740bacac33 100644
  --- a/clang/lib/Sema/OpenCLBuiltins.td
  +++ b/clang/lib/Sema/OpenCLBuiltins.td
  @@ -1692,7 +1692,9 @@ let Extension = FuncExtKhrSubgroups in {
   // --- Table 28.2.2 ---
   let Extension = FuncExtKhrSubgroups in {
 def : Builtin<"sub_group_barrier", [Void, MemFenceFlags], Attr.Convergent>;
  -  def : Builtin<"sub_group_barrier", [Void, MemFenceFlags, MemoryScope], 
Attr.Convergent>;
  +  let MinVersion = CL20 in {
  +def : Builtin<"sub_group_barrier", [Void, MemFenceFlags, MemoryScope], 
Attr.Convergent>;
  +  }
   }
   
   // --- Table 28.2.4 ---


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120254

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

[PATCH] D116514: [clangd] Add code action to generate a constructor for a C++ class

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

In D116514#3311780 , @avogelsgesang 
wrote:

> Can we add this tweak as the default "fix-it" action for
>
>> Class '...' does not declare any constructor to initialize its 
>> non-modifiable members
>
> on structs like
>
>   struct X {
>  int& a;
>   }
>
> ?

Not easy to do in this patch, we don't have a good way to associate fixes with 
clang diagnostics yet.
It's a useful idea but needs a little design I guess.
(-Wswitch kind of works by always marking the tweak as a "fix", which 
semantically doesn't work here)




Comment at: 
clang-tools-extra/clangd/refactor/tweaks/MemberwiseConstructor.cpp:49
+  Class = N->ASTNode.get();
+if (!Class || !Class->isThisDeclarationADefinition() || Class->isUnion())
+  return false;

avogelsgesang wrote:
> njames93 wrote:
> > avogelsgesang wrote:
> > > do we also need to exclude anonymous class declarations here? (Not sure 
> > > if those are also modelled as `CXXRecordDecl` in the clang AST...)
> > Good point, should also ensure there is a test case for this as well.
> On 2nd thought: To trigger this tweak, I click on the class name, and since 
> anonymous structs don't have class names, I think the tweak can't even be 
> triggered.
> 
> Still probably worth a check here, just to be sure. Or at least an `assert`+ 
> comment in the code
Good catch, it's also possible to trigger the code action on the class's braces.
Excluded and added test.



Comment at: 
clang-tools-extra/clangd/refactor/tweaks/MemberwiseConstructor.cpp:133
+// Decide what to do based on the field type.
+class Visitor : public TypeVisitor {
+public:

avogelsgesang wrote:
> Is this visitor able to look through `using MyType = int;` and `typedef`?
> I think we should also add a test case
We call getCanonicalType() before visiting.



Comment at: 
clang-tools-extra/clangd/refactor/tweaks/MemberwiseConstructor.cpp:203
+  return CopyRef;
+return Fail;
+  }

njames93 wrote:
> If C is default constructible, would it be nice to skip here instead of 
> failing?
Yeah, if it's default constructible but *not movable* then it's probably 
reasonable.

(The default constructor should be public, but actually this applies to all 
constructors, so I added this condition everywhere)



Comment at: 
clang-tools-extra/clangd/refactor/tweaks/MemberwiseConstructor.cpp:211
+if (Fields.size() == 1)
+  OS << "explicit ";
+OS << Class->getName() << "(";

njames93 wrote:
> Some codebases may not want these to be explicit, would it be wiser to use 
> config to control this?
I'm not sure that adding an option solves a problem, so I'd rather not add one 
yet.

In most cases, the programmer should be deciding whether the constructor is 
explicit or not on a case-by-case basis. Having a configurable adds complexity 
without relieving significant burden.

We do have to have some default, and explicit for single-arg constructors is 
IMO a better default (recommended by 
[cppcoreguidelines](https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#c46-by-default-declare-single-argument-constructors-explicit),
 [google style 
guide](https://google.github.io/styleguide/cppguide.html#Implicit_Conversions))


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116514

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

[PATCH] D116490: [clangd] Code action to declare missing move/copy constructor/assignment

[Sam McCall via Phabricator via cfe-commits]

sammccall added inline comments.



Comment at: clang-tools-extra/clangd/refactor/tweaks/SpecialMembers.cpp:120
+// they should be =default or =delete.
+Inputs.AST->getSema().ForceDeclarationOfImplicitMembers(Class);
+std::string Code = buildSpecialMemberDeclarations(*Class);

hokein wrote:
> I think we need this because these members are created lazily in clang, e.g. 
> if the empty struct `s` is not used, there is no constructor decl being 
> created.
> 
> The `ForceDeclarationOfImplicitMembers` is a member function which can mutate 
> the parameter `Class`, I was wondering whether it would lead some bad 
> side-effect, but I didn't come out one (and the mutation is mostly creating a 
> new ctor-decl and adding it to the Class).
That's right. Rewrote the comment to make it clearer that these are lazily 
created (which is why I think it's safe to create them)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116490

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

[PATCH] D121992: [Clang] [Driver] Add option to set alternative toolchain path

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added a comment.

In D121992#3418443 , @MaskRay wrote:

> If you intend to overlay ld.so, you'll necessarily overlay libc, then 
> --sysroot seems just unneeded at all.

Why? The header and library search paths are not restricted to artifacts from 
libc. I had consulted some Advance Toolchain developers and they indicted that 
it was important for paths from the `--sysroot` to be incorporated (as GCC from 
the Advance Toolchain does).

The library search order has the issue of needing the multiarch/"OS lib dir" 
paths from both the overlay toolchain and the sysroot before attempting 
fallback plain-"lib" paths. Aside from a new option, is there a way to achieve 
that?

I agree, however, that the new option doesn't have to incorporate effects on 
what the `--dyld-prefix` default is.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121992

___
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 created this revision.
ymandel added reviewers: sgatev, gribozavr2.
Herald added subscribers: tschuett, steakhal.
Herald added a project: All.
ymandel updated this revision to Diff 417906.
ymandel added a comment.
ymandel updated this revision to Diff 419430.
ymandel retitled this revision from "[clang][dataflow] Fields bug repro. NOT 
FOR REVIEW." to "[clang][dataflow] Fix handling of base-class fields".
ymandel edited the summary of this revision.
ymandel updated this revision to Diff 419440.
ymandel edited the summary of this revision.
ymandel updated this revision to Diff 419449.
ymandel updated this revision to Diff 419451.
ymandel added a reviewer: xazax.hun.
Herald added a subscriber: rnkovacs.
ymandel published this revision for review.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Added repros for equality-related failures.


ymandel added a comment.

c


ymandel added a comment.

added missing call


ymandel added a comment.

refactor


ymandel added a comment.

tweak


This patch adds support for tracking of non-private base-class fields in 
derived classes.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D122273

Files:
  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,59 @@
   });
 }
 
+TEST_F(TransferTest, DerivedBaseMember) {
+  std::string Code = R"(
+struct A {
+  int Bar;
+};
+struct B : public A {
+};
+
+void target(B Foo) {
+  int Baz = Foo.Bar;
+  // [[p]]
+}
+  )";
+  runDataflow(
+  Code, [](llvm::ArrayRef<
+   std::pair>>
+   Results,
+   ASTContext &ASTCtx) {
+ASSERT_THAT(Results, ElementsAre(Pair("p", _)));
+const Environment &Env = Results[0].second.Env;
+
+const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
+ASSERT_THAT(FooDecl, NotNull());
+
+ASSERT_TRUE(FooDecl->getType()->isStructureType());
+const FieldDecl *BarDecl = nullptr;
+for (const clang::CXXBaseSpecifier &Base :
+ FooDecl->getType()->getAsCXXRecordDecl()->bases()) {
+  QualType BaseType = Base.getType();
+  ASSERT_TRUE(BaseType->isStructureType());
+
+  for (const FieldDecl *Field : BaseType->getAsRecordDecl()->fields()) {
+if (Field->getNameAsString() == "Bar") {
+  BarDecl = Field;
+} else {
+  FAIL() << "Unexpected field: " << Field->getNameAsString();
+}
+  }
+}
+ASSERT_THAT(BarDecl, NotNull());
+
+const auto *FooLoc = cast(
+Env.getStorageLocation(*FooDecl, SkipPast::None));
+const auto *FooVal = cast(Env.getValue(*FooLoc));
+const auto *BarVal = cast(FooVal->getChild(*BarDecl));
+
+const ValueDecl *BazDecl = findValueDecl(ASTCtx, "Baz");
+ASSERT_THAT(BazDecl, NotNull());
+
+EXPECT_EQ(Env.getValue(*BazDecl, SkipPast::None), BarVal);
+  });
+}
+
 TEST_F(TransferTest, ClassMember) {
   std::string Code = R"(
 class A {
Index: clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
===
--- clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
+++ clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
@@ -164,6 +164,39 @@
   return JoinedConstraints;
 }
 
+static void
+getFieldsFromClassHierarchy(QualType Type, bool IgnorePrivateFields,
+llvm::DenseSet &Fields) {
+  if (Type->isIncompleteType() || Type->isDependentType() ||
+  !Type->isRecordType())
+return;
+
+  for (const FieldDecl *Field : Type->getAsRecordDecl()->fields()) {
+if (IgnorePrivateFields && (Field->getAccess() == clang::AS_private ||
+(Field->getAccess() == clang::AS_none &&
+ Type->getAsRecordDecl()->isClass(
+  continue;
+Fields.insert(Field);
+  }
+  if (auto *CXXRecord = Type->getAsCXXRecordDecl()) {
+for (const clang::CXXBaseSpecifier &Base : CXXRecord->bases()) {
+  // Ignore private fields (including default access in C++ classes) in
+  // base classes, because they are not visible in derived classes.
+  getFieldsFromClassHierarchy(Base.getType(), /*IgnorePrivateFields=*/true,
+  Fields);
+}
+  }
+}
+
+/// Gets the set of all fields accesible from the type.
+static llvm::DenseSet
+getAccessibleObjectFields(QualType Type) {
+  llvm::DenseSet Fields;
+  // Don't ignore private fields for the class itself, only its super classes.
+  getFieldsFromClassHierarchy(Type, /*IgnorePrivateFields=*/false, Field

[PATCH] D120254: [OpenCL] Align subgroup builtin guards

[Harald van Dijk via Phabricator via cfe-commits]

hvdijk added a comment.

In D120254#3419369 , @svenvh wrote:

> In D120254#3419221 , @hvdijk wrote:
>
>> The problem is that this change enables certain built-ins in OpenCL 1.2 that 
>> take a memory_scope argument, but the memory_scope type is not defined in 
>> OpenCL 1.2 mode. When we then process the function, sub_group_barrier in my 
>> example, things break when checking whether the declaration matches the 
>> built-in. I am not sure what the right fix here is. Can we just define the 
>> type if any extension is enabled that requires the type, or is that not 
>> allowed?
>
> Thanks for digging further and providing a reproducer!  I think the fix is to 
> only make the `sub_group_barrier(cl_mem_fence_flags flags, memory_scope)` 
> overload available for OpenCL 2.0 or above.  That would also match 
> `opencl-c.h`.
>
> The following patch seems to fix the issue that you described:

Huh, I could have sworn I saw more builtins than just this one but clearly I 
messed up there, that's the only one. That fix looks good to me, thanks, do you 
want to submit that as a new change along with the test or would you like a 
hand with it?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120254

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

[PATCH] D118050: [analyzer] Different address spaces cannot overlap

[Vince Bridgers via Phabricator via cfe-commits]

vabridgers planned changes to this revision.
vabridgers added a comment.

I need to make a few changes. Please hold off any comments until the next 
patch, coming soon.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118050

___
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 created this revision.
erichkeane added reviewers: rjmccall, aaron.ballman, tahonermann.
Herald added a project: All.
erichkeane requested review of this revision.

As reported in https://github.com/llvm/llvm-project/issues/54588
and discussed in https://github.com/itanium-cxx-abi/cxx-abi/issues/139

We are supposed to do a DFS, pre-order, decl-order search for a name for
the union in this case. Prevoiusly we crashed because the IdentiferInfo
pointer was nullptr, so this makes sure we have a name in the cases
described by the ABI.

I added an llvm-unreachable to cover an unexpected case at the end of
the new function with information/reference to the ABI in case we come
up with some way to get back to here.


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,99 @@
+// 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 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{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}}}>()()
+}
Index: clang/lib/AST/ItaniumMangle.cpp
===
--- clang/lib/AST/ItaniumMangle.cpp
+++ clang/lib/AST/ItaniumMangle.cpp
@@ -5545,6 +5545,37 @@
   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.
+  const CXXRecordDecl *RD = FD->getType()->getAsCXXRecordDecl();
+
+  // Consider only the fields in declaration order, searched depth-first.  We
+  // don't care about the active member of the union, as all we are doing is
+  // looking for a valid name. We also don't check bases, due to guid

[PATCH] D122789: [compiler-rt] [scudo] Use -mcrc32 on x86 when available

[Phoebe Wang via Phabricator via cfe-commits]

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

LGTM, thanks!


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

https://reviews.llvm.org/D122789

___
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 added a comment.

Ping!


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 created this revision.
yihanaa added reviewers: aaron.ballman, erichkeane, MaskRay.
yihanaa added a project: clang.
Herald added a subscriber: StephenFan.
Herald added a project: All.
yihanaa requested review of this revision.
Herald added a subscriber: cfe-commits.

Add constant array support for __builtin_dump_sturct. For N-dimensional arrays, 
the new dumpArray function dumps the elements of the array by generating N 
layers of 'for' loops.
for example:
The struct:

  struct Foo {
int x;
  };
  
  struct S {
int a[3];
int b[2][2];
struct Foo c[2];
  };

The dump result:

  struct S {
  int[3] a = [
  [0] = 1
  [1] = 2
  [2] = 3
  ]
  int[2][2] b = [
  [0] = [
  [0] = 1
  [1] = 2
  ]
  [1] = [
  [0] = 3
  [1] = 4
  ]
  ]
  struct Foo[2] c = [
  [0] = {
  int x = 1
  }
  [1] = {
  int x = 2
  }
  ]
  }


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D122822

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/test/CodeGen/dump-struct-builtin.c

Index: clang/test/CodeGen/dump-struct-builtin.c
===
--- clang/test/CodeGen/dump-struct-builtin.c
+++ clang/test/CodeGen/dump-struct-builtin.c
@@ -76,7 +76,11 @@
 
 // CHECK: @__const.unit15.a = private unnamed_addr constant %struct.U15A { [3 x i32] [i32 1, i32 2, i32 3] }, align 4
 // CHECK-NEXT: [[STRUCT_STR_U15:@[0-9]+]] = private unnamed_addr constant [15 x i8] c"struct U15A {\0A\00", align 1
-// CHECK-NEXT: [[FIELD_U15:@[0-9]+]] = private unnamed_addr constant [19 x i8] c"int[3] a = %p\0A\00", align 1
+// CHECK-NEXT: [[FIELD_U15:@[0-9]+]] = private unnamed_addr constant [16 x i8] c"int[3] a = \00", align 1
+// CHECK-NEXT: [[ARRAY_L_SQUARE_U15:@[0-9]+]] = private unnamed_addr constant [3 x i8] c"[\0A\00", align 1
+// CHECK-NEXT: [[ARRAY_INDEX_U15:@[0-9]+]] = private unnamed_addr constant [17 x i8] c"[%ld] = \00", align 1
+// CHECK-NEXT: [[ARRAY_ELEMENT_U15:@[0-9]+]] = private unnamed_addr constant [4 x i8] c"%d\0A\00", align 1
+// CHECK-NEXT: [[ARRAY_R_SQUARE_U15:@[0-9]+]] = private unnamed_addr constant [7 x i8] c"]\0A\00", align 1
 // CHECK-NEXT: [[END_STRUCT_U15:@[0-9]+]] = private unnamed_addr constant [3 x i8] c"}\0A\00", align 1
 
 // CHECK: @__const.unit16.a = private unnamed_addr constant %struct.U16A { i8 12 }, align 1
@@ -94,6 +98,18 @@
 // CHECK-NEXT: [[FIELD_U18:@[0-9]+]] = private unnamed_addr constant [25 x i8] c"long double a = %Lf\0A\00", align 1
 // CHECK-NEXT: [[END_STRUCT_U18:@[0-9]+]] = private unnamed_addr constant [3 x i8] c"}\0A\00", align 1
 
+// CHECK: @__const.unit19.a = private unnamed_addr constant %struct.T19A { {{.*}} }, align 4
+// CHECK-NEXT: [[STRUCT_STR_U19:@[0-9]+]] = private unnamed_addr constant [15 x i8] c"struct T19A {\0A\00", align 1
+// CHECK-NEXT: [[FIELD_U19:@[0-9]+]] = private unnamed_addr constant [19 x i8] c"int[2][2] b = \00", align 1
+// CHECK-NEXT: [[ARRAY_L_SQUARE_U19:@[0-9]+]] = private unnamed_addr constant [3 x i8] c"[\0A\00", align 1
+// CHECK-NEXT: [[ARRAY_INDEX_U19:@[0-9]+]] = private unnamed_addr constant [17 x i8] c"[%ld] = \00", align 1
+// CHECK-NEXT: [[ARRAY_L_SQUARE2_U19:@[0-9]+]] = private unnamed_addr constant [3 x i8] c"[\0A\00", align 1
+// CHECK-NEXT: [[ARRAY_INDEX2_U19:@[0-9]+]] = private unnamed_addr constant [21 x i8] c"[%ld] = \00", align 1
+// CHECK-NEXT: [[ARRAY_ELEMENT_U19:@[0-9]+]] = private unnamed_addr constant [4 x i8] c"%d\0A\00", align 1
+// CHECK-NEXT: [[ARRAY_R_SQUARE2_U19:@[0-9]+]] = private unnamed_addr constant [11 x i8] c"]\0A\00", align 1
+// CHECK-NEXT: [[ARRAY_R_SQUARE_U19:@[0-9]+]] = private unnamed_addr constant [7 x i8] c"]\0A\00", align 1
+// CHECK-NEXT: [[END_STRUCT_U19:@[0-9]+]] = private unnamed_addr constant [3 x i8] c"}\0A\00", align 1
+
 int printf(const char *fmt, ...) {
 return 0;
 }
@@ -107,8 +123,8 @@
   .a = 12,
   };
   // CHECK: call i32 (i8*, ...) @printf(i8* getelementptr inbounds ([14 x i8], [14 x i8]* [[STRUCT_STR_U1]], i32 0, i32 0))
-  // CHECK: [[RES1:%.*]] = getelementptr inbounds %struct.U1A, %struct.U1A* %a, i32 0, i32 0
-  // CHECK: [[LOAD1:%.*]] = load i16, i16* [[RES1]],
+  // CHECK: [[RES1:%[−a−zA−Z._0-9]+]] = getelementptr inbounds %struct.U1A, %struct.U1A* %a, i32 0, i32 0
+  // CHECK: [[LOAD1:%[0-9]+]] = load i16, i16* [[RES1]],
   // CHECK: call i32 (i8*, ...) @printf(i8* getelementptr inbounds ([19 x i8], [19 x i8]* [[FIELD_U1]], i32 0, i32 0), i16 [[LOAD1]])
   // CHECK: call i32 (i8*, ...) @printf(i8* getelementptr inbounds ([3 x i8], [3 x i8]* [[END_STRUCT_U1]], i32 0, i32 0))
   __builtin_dump_struct(&a, &printf);
@@ -124,8 +140,8 @@
   };
 
   // CHECK: call i32 (i8*, ...) @printf(i8* getelementptr inbounds ([14 x i8], [14 x i8]* [[STRUCT_STR_U2]], i3

[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.
This revision is now accepted and ready to land.



Comment at: clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp:168
+static void
+getFieldsFromClassHierarchy(QualType Type, bool IgnorePrivateFields,
+llvm::DenseSet &Fields) {

Let's add to the documentation of `AggregateStorageLocation` and `StructValue` 
that they implement a flat struct layout. I don't see an immediate reason to 
revisit this, but let's be explicit about it.



Comment at: clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp:182
+  if (auto *CXXRecord = Type->getAsCXXRecordDecl()) {
+for (const clang::CXXBaseSpecifier &Base : CXXRecord->bases()) {
+  // Ignore private fields (including default access in C++ classes) in

The `clang` namespace is unnecessary.



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

Add a similar test with `class` instead of `struct`?



Comment at: clang/unittests/Analysis/FlowSensitive/TransferTest.cpp:1015
+struct A {
+  int Bar;
+};

Let's also add private and protected members in `A` and a private member in `B`.


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] D118050: [analyzer] Different address spaces cannot overlap

[Vince Bridgers via Phabricator via cfe-commits]

vabridgers updated this revision to Diff 419462.
vabridgers added a comment.
This revision is now accepted and ready to land.

make assert function void, update summary


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118050

Files:
  clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
  clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
  clang/test/Analysis/cstring-addrspace.c

Index: clang/test/Analysis/cstring-addrspace.c
===
--- /dev/null
+++ clang/test/Analysis/cstring-addrspace.c
@@ -0,0 +1,64 @@
+// RUN: %clang_analyze_cc1 -triple amdgcn-unknown-unknown \
+// RUN: -analyze -analyzer-checker=core,alpha.unix.cstring \
+// RUN: -analyze -analyzer-checker=debug.ExprInspection \
+// RUN: -analyzer-config crosscheck-with-z3=true -verify %s \
+// RUN: -Wno-incompatible-library-redeclaration
+// REQUIRES: z3
+
+void clang_analyzer_warnIfReached();
+
+// From https://llvm.org/docs/AMDGPUUsage.html#address-spaces,
+// select address space 3 (local), since the pointer size is
+// different than Generic.
+#define DEVICE __attribute__((address_space(3)))
+_Static_assert(sizeof(int *) == 8, "");
+_Static_assert(sizeof(DEVICE int *) == 4, "");
+_Static_assert(sizeof(void *) == 8, "");
+_Static_assert(sizeof(DEVICE void *) == 4, "");
+
+// Copy from host to device memory. Note this is specialized
+// since one of the parameters is assigned an address space such
+// that the sizeof the the pointer is different than the other.
+//
+// Some downstream implementations may have specialized memcpy
+// routines that copy from one address space to another. In cases
+// like that, the address spaces are assumed to not overlap, so the
+// cstring overlap check is not needed. When a static analysis report
+// is generated in as case like this, SMTConv may attempt to create
+// a refutation to Z3 with different bitwidth pointers which lead to
+// a crash. This is not common in directly used upstream compiler builds,
+// but can be seen in specialized downstrean implementations. This case
+// covers those specific instances found and debugged.
+//
+// Since memcpy is a builtin, a specialized builtin instance named like
+// 'memcpy_special' will hit in cstring, triggering this behavior. The
+// best we can do for an upstream test is use the same memcpy function name.
+DEVICE void *memcpy(DEVICE void *dst, const void *src, unsigned long len);
+
+void top1(DEVICE void *dst, void *src, int len) {
+  memcpy(dst, src, len);
+
+  // Create a bugreport for triggering Z3 refutation.
+  clang_analyzer_warnIfReached(); // expected-warning {{REACHABLE}}
+}
+
+void top2(DEVICE int *dst, void *src, int len) {
+  memcpy(dst, src, len);
+
+  // Create a bugreport for triggering Z3 refutation.
+  clang_analyzer_warnIfReached(); // expected-warning {{REACHABLE}}
+}
+
+void top3(DEVICE int *dst, int *src, int len) {
+  memcpy(dst, src, len);
+
+  // Create a bugreport for triggering Z3 refutation.
+  clang_analyzer_warnIfReached(); // expected-warning {{REACHABLE}}
+}
+
+void top4() {
+  memcpy((DEVICE void *)1, (const void *)1, 1);
+
+  // Create a bugreport for triggering Z3 refutation.
+  clang_analyzer_warnIfReached(); // expected-warning {{REACHABLE}}
+}
Index: clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
===
--- clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
+++ clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
@@ -715,11 +715,41 @@
   llvm_unreachable("Fields not found in parent record's definition");
 }
 
+// This is used in debug builds only for now because some downstream users
+// may hit this assert in their subsequent merges.
+// There are still places in the analyzer where equal bitwidth Locs
+// are compared, and need to be found and corrected. Recent previous fixes have
+// addressed the known problems of making NULLs with specific bitwidths
+// for Loc comparisons along with deprecation of APIs for the same purpose.
+//
+static void assertEqualBitWidths(ProgramStateRef State, Loc RhsLoc,
+ Loc LhsLoc) {
+  // Implements a "best effort" check for RhsLoc and LhsLoc bit widths
+  ASTContext &Ctx = State->getStateManager().getContext();
+  uint64_t RhsBitwidth =
+  RhsLoc.getType(Ctx).isNull() ? 0 : Ctx.getTypeSize(RhsLoc.getType(Ctx));
+  uint64_t LhsBitwidth =
+  LhsLoc.getType(Ctx).isNull() ? 0 : Ctx.getTypeSize(LhsLoc.getType(Ctx));
+  if (RhsBitwidth && LhsBitwidth &&
+  (LhsLoc.getSubKind() == RhsLoc.getSubKind())) {
+assert(RhsBitwidth == LhsBitwidth &&
+   "RhsLoc and LhsLoc bitwidth must be same!");
+  }
+}
+
 // FIXME: all this logic will change if/when we have MemRegion::getLocation().
 SVal SimpleSValBuilder::evalBinOpLL(ProgramStateRef state,
   BinaryOperator::Opcode op,
   Lo

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

[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/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] 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.

Is the OpenMP lit test missing?


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

[clang] d9739f2 - Serialize PragmaAssumeNonNullLoc to support preambles

[David Goldman via cfe-commits]

Author: David Goldman
Date: 2022-03-31T11:08:01-04:00
New Revision: d9739f29cdd4c066763275d09e1d26ee315cfdf5

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

LOG: Serialize PragmaAssumeNonNullLoc to support preambles

Previously, if a `#pragma clang assume_nonnull begin` was at the
end of a premable with a `#pragma clang assume_nonnull end` at the
end of the main file, clang would diagnose an unterminated begin in
the preamble and an unbalanced end in the main file.

With this change, those errors no longer occur and the case above is
now properly handled. I've added a corresponding test to clangd,
which makes use of preambles, in order to verify this works as
expected.

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

Added: 
clang/test/Index/preamble-assume-nonnull.c

Modified: 
clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
clang/include/clang/Lex/Preprocessor.h
clang/include/clang/Lex/PreprocessorOptions.h
clang/include/clang/Serialization/ASTBitCodes.h
clang/lib/Lex/PPLexerChange.cpp
clang/lib/Serialization/ASTReader.cpp
clang/lib/Serialization/ASTWriter.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp 
b/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
index 097ada51b2e9a..5a5a53679facc 100644
--- a/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
+++ b/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
@@ -747,6 +747,40 @@ TEST(DiagnosticsTest, RecursivePreambleIfndefGuard) {
   EXPECT_THAT(TU.build().getLocalTopLevelDecls(), SizeIs(1));
 }
 
+TEST(DiagnosticsTest, PreambleWithPragmaAssumeNonnull) {
+  auto TU = TestTU::withCode(R"cpp(
+#pragma clang assume_nonnull begin
+void foo(int *x);
+#pragma clang assume_nonnull end
+)cpp");
+  auto AST = TU.build();
+  EXPECT_THAT(*AST.getDiagnostics(), IsEmpty());
+  const auto *X = cast(findDecl(AST, "foo")).getParamDecl(0);
+  ASSERT_TRUE(X->getOriginalType()->getNullability(X->getASTContext()) ==
+  NullabilityKind::NonNull);
+}
+
+TEST(DiagnosticsTest, PreambleHeaderWithBadPragmaAssumeNonnull) {
+  Annotations Header(R"cpp(
+#pragma clang assume_nonnull begin  // error-ok
+void foo(int *X);
+)cpp");
+  auto TU = TestTU::withCode(R"cpp(
+#include "foo.h"  // unterminated assume_nonnull should not affect bar.
+void bar(int *Y);
+)cpp");
+  TU.AdditionalFiles = {{"foo.h", std::string(Header.code())}};
+  auto AST = TU.build();
+  EXPECT_THAT(*AST.getDiagnostics(),
+  ElementsAre(diagName("pp_eof_in_assume_nonnull")));
+  const auto *X = cast(findDecl(AST, "foo")).getParamDecl(0);
+  ASSERT_TRUE(X->getOriginalType()->getNullability(X->getASTContext()) ==
+  NullabilityKind::NonNull);
+  const auto *Y = cast(findDecl(AST, "bar")).getParamDecl(0);
+  ASSERT_FALSE(
+  Y->getOriginalType()->getNullability(X->getASTContext()).hasValue());
+}
+
 TEST(DiagnosticsTest, InsideMacros) {
   Annotations Test(R"cpp(
 #define TEN 10

diff  --git a/clang/include/clang/Lex/Preprocessor.h 
b/clang/include/clang/Lex/Preprocessor.h
index 36bf8ed64c2bc..02b94872cd8ae 100644
--- a/clang/include/clang/Lex/Preprocessor.h
+++ b/clang/include/clang/Lex/Preprocessor.h
@@ -409,6 +409,14 @@ class Preprocessor {
   /// \#pragma clang assume_nonnull begin.
   SourceLocation PragmaAssumeNonNullLoc;
 
+  /// Set only for preambles which end with an active
+  /// \#pragma clang assume_nonnull begin.
+  ///
+  /// When the preamble is loaded into the main file,
+  /// `PragmaAssumeNonNullLoc` will be set to this to
+  /// replay the unterminated assume_nonnull.
+  SourceLocation PreambleRecordedPragmaAssumeNonNullLoc;
+
   /// True if we hit the code-completion point.
   bool CodeCompletionReached = false;
 
@@ -1762,6 +1770,21 @@ class Preprocessor {
 PragmaAssumeNonNullLoc = Loc;
   }
 
+  /// Get the location of the recorded unterminated \#pragma clang
+  /// assume_nonnull begin in the preamble, if one exists.
+  ///
+  /// Returns an invalid location if the premable did not end with
+  /// such a pragma active or if there is no recorded preamble.
+  SourceLocation getPreambleRecordedPragmaAssumeNonNullLoc() const {
+return PreambleRecordedPragmaAssumeNonNullLoc;
+  }
+
+  /// Record the location of the unterminated \#pragma clang
+  /// assume_nonnull begin in the preamble.
+  void setPreambleRecordedPragmaAssumeNonNullLoc(SourceLocation Loc) {
+PreambleRecordedPragmaAssumeNonNullLoc = Loc;
+  }
+
   /// Set the directory in which the main file should be considered
   /// to have been found, if it is not a real file.
   void setMainFileDir(const DirectoryEntry *Dir) {

diff  --git a/clang/include/clang/Lex/PreprocessorOptions.h 
b/clang/include/clang/Lex/PreprocessorOptions.h
index a7aabc3e1df2a..dc5382

[PATCH] D119544: Deferred Concept Instantiation Implementation

[Erich Keane via Phabricator via cfe-commits]

erichkeane updated this revision to Diff 419463.
erichkeane marked 5 inline comments as done.
erichkeane added a comment.

Respond to @ChuanqiXu  and fix a few comments he suggested.


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

https://reviews.llvm.org/D119544

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Sema/Sema.h
  clang/include/clang/Sema/Template.h
  clang/lib/Sema/SemaConcept.cpp
  clang/lib/Sema/SemaTemplate.cpp
  clang/lib/Sema/SemaTemplateDeduction.cpp
  clang/lib/Sema/SemaTemplateInstantiate.cpp
  clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
  clang/lib/Sema/TreeTransform.h
  clang/test/CXX/temp/temp.constr/temp.constr.constr/non-function-templates.cpp
  clang/test/SemaTemplate/deferred-concept-inst.cpp
  clang/test/SemaTemplate/instantiate-requires-clause.cpp
  clang/test/SemaTemplate/trailing-return-short-circuit.cpp

Index: clang/test/SemaTemplate/trailing-return-short-circuit.cpp
===
--- /dev/null
+++ clang/test/SemaTemplate/trailing-return-short-circuit.cpp
@@ -0,0 +1,61 @@
+// RUN: %clang_cc1 -std=c++20 -verify %s
+
+template 
+requires(sizeof(T) > 2) || T::value // #FOO_REQ
+void Foo(T){};  // #FOO
+
+template 
+void TrailingReturn(T) // #TRAILING
+requires(sizeof(T) > 2) || // #TRAILING_REQ
+T::value{};// #TRAILING_REQ_VAL
+template 
+struct HasValue {
+  static constexpr bool value = B;
+};
+static_assert(sizeof(HasValue) <= 2);
+
+template 
+struct HasValueLarge {
+  static constexpr bool value = B;
+  int I;
+};
+static_assert(sizeof(HasValueLarge) > 2);
+
+void usage() {
+  // Passes the 1st check, short-circuit so the 2nd ::value is not evaluated.
+  Foo(1.0);
+  TrailingReturn(1.0);
+
+  // Fails the 1st check, but has a ::value, so the check happens correctly.
+  Foo(HasValue{});
+  TrailingReturn(HasValue{});
+
+  // Passes the 1st check, but would have passed the 2nd one.
+  Foo(HasValueLarge{});
+  TrailingReturn(HasValueLarge{});
+
+  // Fails the 1st check, fails 2nd because there is no ::value.
+  Foo(true);
+  // expected-error@-1{{no matching function for call to 'Foo'}}
+  // expected-note@#FOO{{candidate template ignored: constraints not satisfied [with T = bool]}}
+  // expected-note@#FOO_REQ{{because 'sizeof(_Bool) > 2' (1 > 2) evaluated to false}}
+  // expected-note@#FOO_REQ{{because substituted constraint expression is ill-formed: type 'bool' cannot be used prior to '::' because it has no members}}
+
+  TrailingReturn(true);
+  // expected-error@-1{{no matching function for call to 'TrailingReturn'}}
+  // expected-note@#TRAILING{{candidate template ignored: constraints not satisfied [with T = bool]}}
+  // expected-note@#TRAILING_REQ{{because 'sizeof(_Bool) > 2' (1 > 2) evaluated to false}}
+  // expected-note@#TRAILING_REQ_VAL{{because substituted constraint expression is ill-formed: type 'bool' cannot be used prior to '::' because it has no members}}
+
+  // Fails the 1st check, fails 2nd because ::value is false.
+  Foo(HasValue{});
+  // expected-error@-1 {{no matching function for call to 'Foo'}}
+  // expected-note@#FOO{{candidate template ignored: constraints not satisfied [with T = HasValue]}}
+  // expected-note@#FOO_REQ{{because 'sizeof(HasValue) > 2' (1 > 2) evaluated to false}}
+  // expected-note@#FOO_REQ{{and 'HasValue::value' evaluated to false}}
+  TrailingReturn(HasValue{});
+  // expected-error@-1 {{no matching function for call to 'TrailingReturn'}}
+  // expected-note@#TRAILING{{candidate template ignored: constraints not satisfied [with T = HasValue]}}
+  // expected-note@#TRAILING_REQ{{because 'sizeof(HasValue) > 2' (1 > 2) evaluated to false}}
+  // expected-note@#TRAILING_REQ_VAL{{and 'HasValue::value' evaluated to false}}
+}
Index: clang/test/SemaTemplate/instantiate-requires-clause.cpp
===
--- clang/test/SemaTemplate/instantiate-requires-clause.cpp
+++ clang/test/SemaTemplate/instantiate-requires-clause.cpp
@@ -40,6 +40,18 @@
 
 static_assert(S::f(1));
 
+// Similar to the 'S' test, but tries to use 'U' in the requires clause.
+template 
+struct S1 {
+  // expected-note@+3 {{candidate template ignored: constraints not satisfied [with U = int]}}
+  // expected-note@+2 {{because substituted constraint expression is ill-formed: type 'int' cannot be used prior to '::' because it has no members}}
+  template 
+  static constexpr auto f(U const index) requires(U::foo) { return true; }
+};
+
+// expected-error@+1 {{no matching function for call to 'f'}}
+static_assert(S1::f(1));
+
 constexpr auto value = 0;
 
 template
Index: clang/test/SemaTemplate/deferred-concept-inst.cpp
===
--- /dev/null
+++ clang/test/SemaTemplate/deferred-concept-inst.cpp
@@ -0,0 +1,23 @@
+// RUN: %clang_cc1 -std=c++2a -x c++ %s -verify -Wno-unused-value
+// expected-no-diagnostics
+
+namespace

[PATCH] D122179: Serialize PragmaAssumeNonNullLoc to support preambles

[David Goldman via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGd9739f29cdd4: Serialize PragmaAssumeNonNullLoc to support 
preambles (authored by dgoldman).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122179

Files:
  clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
  clang/include/clang/Lex/Preprocessor.h
  clang/include/clang/Lex/PreprocessorOptions.h
  clang/include/clang/Serialization/ASTBitCodes.h
  clang/lib/Lex/PPLexerChange.cpp
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTWriter.cpp
  clang/test/Index/preamble-assume-nonnull.c

Index: clang/test/Index/preamble-assume-nonnull.c
===
--- /dev/null
+++ clang/test/Index/preamble-assume-nonnull.c
@@ -0,0 +1,6 @@
+// RUN: env CINDEXTEST_EDITING=1 c-index-test -test-load-source local %s 2>&1 \
+// RUN: | FileCheck %s --implicit-check-not "error:"
+
+#pragma clang assume_nonnull begin
+void foo(int *x);
+#pragma clang assume_nonnull end
Index: clang/lib/Serialization/ASTWriter.cpp
===
--- clang/lib/Serialization/ASTWriter.cpp
+++ clang/lib/Serialization/ASTWriter.cpp
@@ -872,6 +872,7 @@
   RECORD(PP_CONDITIONAL_STACK);
   RECORD(DECLS_TO_CHECK_FOR_DEFERRED_DIAGS);
   RECORD(PP_INCLUDED_FILES);
+  RECORD(PP_ASSUME_NONNULL_LOC);
 
   // SourceManager Block.
   BLOCK(SOURCE_MANAGER_BLOCK);
@@ -2299,6 +2300,17 @@
 Stream.EmitRecord(PP_COUNTER_VALUE, Record);
   }
 
+  // If we have a recorded #pragma assume_nonnull, remember it so it can be
+  // replayed when the preamble terminates into the main file.
+  SourceLocation AssumeNonNullLoc =
+  PP.getPreambleRecordedPragmaAssumeNonNullLoc();
+  if (AssumeNonNullLoc.isValid()) {
+assert(PP.isRecordingPreamble());
+AddSourceLocation(AssumeNonNullLoc, Record);
+Stream.EmitRecord(PP_ASSUME_NONNULL_LOC, Record);
+Record.clear();
+  }
+
   if (PP.isRecordingPreamble() && PP.hasRecordedPreamble()) {
 assert(!IsModule);
 auto SkipInfo = PP.getPreambleSkipInfo();
Index: clang/lib/Serialization/ASTReader.cpp
===
--- clang/lib/Serialization/ASTReader.cpp
+++ clang/lib/Serialization/ASTReader.cpp
@@ -3109,6 +3109,7 @@
   case IDENTIFIER_OFFSET:
   case INTERESTING_IDENTIFIERS:
   case STATISTICS:
+  case PP_ASSUME_NONNULL_LOC:
   case PP_CONDITIONAL_STACK:
   case PP_COUNTER_VALUE:
   case SOURCE_LOCATION_OFFSETS:
@@ -3370,6 +3371,14 @@
   }
   break;
 
+case PP_ASSUME_NONNULL_LOC: {
+  unsigned Idx = 0;
+  if (!Record.empty())
+PP.setPreambleRecordedPragmaAssumeNonNullLoc(
+ReadSourceLocation(F, Record, Idx));
+  break;
+}
+
 case PP_CONDITIONAL_STACK:
   if (!Record.empty()) {
 unsigned Idx = 0, End = Record.size() - 1;
Index: clang/lib/Lex/PPLexerChange.cpp
===
--- clang/lib/Lex/PPLexerChange.cpp
+++ clang/lib/Lex/PPLexerChange.cpp
@@ -432,8 +432,13 @@
   // instantiation or a _Pragma.
   if (PragmaAssumeNonNullLoc.isValid() &&
   !isEndOfMacro && !(CurLexer && CurLexer->Is_PragmaLexer)) {
-Diag(PragmaAssumeNonNullLoc, diag::err_pp_eof_in_assume_nonnull);
-
+// If we're at the end of generating a preamble, we should record the
+// unterminated \#pragma clang assume_nonnull so we can restore it later
+// when the preamble is loaded into the main file.
+if (isRecordingPreamble() && isInPrimaryFile())
+  PreambleRecordedPragmaAssumeNonNullLoc = PragmaAssumeNonNullLoc;
+else
+  Diag(PragmaAssumeNonNullLoc, diag::err_pp_eof_in_assume_nonnull);
 // Recover by leaving immediately.
 PragmaAssumeNonNullLoc = SourceLocation();
   }
@@ -514,10 +519,14 @@
  PPCallbacks::ExitFile, FileType, ExitedFID);
 }
 
-// Restore conditional stack from the preamble right after exiting from the
-// predefines file.
-if (ExitedFromPredefinesFile)
+// Restore conditional stack as well as the recorded
+// \#pragma clang assume_nonnull from the preamble right after exiting
+// from the predefines file.
+if (ExitedFromPredefinesFile) {
   replayPreambleConditionalStack();
+  if (PreambleRecordedPragmaAssumeNonNullLoc.isValid())
+PragmaAssumeNonNullLoc = PreambleRecordedPragmaAssumeNonNullLoc;
+}
 
 if (!isEndOfMacro && CurPPLexer && FoundPCHThroughHeader &&
 (isInPrimaryFile() ||
Index: clang/include/clang/Serialization/ASTBitCodes.h
===
--- clang/include/clang/Serialization/ASTBitCodes.h
+++ clang/include/clang/Serialization/ASTBitCodes.h
@@ -698,6 +698,10 @@
 
   /// Record code for included files.
   PP_INCLUDED_FILES = 66,
+
+  /// 

[PATCH] D118050: [analyzer] Avoid checking addrspace pointers in cstring checker

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

steakhal accepted this revision.
steakhal added a comment.

Looks great!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118050

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

[PATCH] D120306: [clangd] IncludeCleaner: Add support for IWYU pragma private

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

Looks like this breaks tests on Windows: http://45.33.8.238/win/55402/step_9.txt

Please take a look, and revert for now if it takes a while to fix.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120306

___
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

[Sam McCall via Phabricator via cfe-commits]

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

@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 
:-)


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] D122273: [clang][dataflow] Fix handling of base-class fields

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

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);

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.


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] D122830: [clang][dataflow] Add support for (built-in) (in)equality operators

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

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

Adds logical interpretation of built-in equality operators, `==` and `!=`.s


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D122830

Files:
  clang/lib/Analysis/FlowSensitive/Transfer.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
@@ -2410,4 +2410,74 @@
   });
 }
 
+TEST_F(TransferTest, Equality) {
+  std::string Code = R"(
+void target(bool Bar) {
+  bool Foo = true;
+  if (Bar == Foo) {
+(void)0;
+/*[[p-then]]*/
+  } else {
+(void)0;
+/*[[p-else]]*/
+  }
+}
+  )";
+  runDataflow(
+  Code, [](llvm::ArrayRef<
+   std::pair>>
+   Results,
+   ASTContext &ASTCtx) {
+ASSERT_THAT(Results, ElementsAre(Pair("p-else", _), Pair("p-then", _)));
+const Environment &EnvElse = Results[0].second.Env;
+const Environment &EnvThen = Results[1].second.Env;
+
+const ValueDecl *BarDecl = findValueDecl(ASTCtx, "Bar");
+ASSERT_THAT(BarDecl, NotNull());
+
+auto &BarValThen =
+*cast(EnvThen.getValue(*BarDecl, SkipPast::None));
+EXPECT_TRUE(EnvThen.flowConditionImplies(BarValThen));
+
+auto &BarValElse =
+*cast(EnvElse.getValue(*BarDecl, SkipPast::None));
+EXPECT_FALSE(EnvElse.flowConditionImplies(BarValElse));
+  });
+}
+
+TEST_F(TransferTest, Inequality) {
+  std::string Code = R"(
+void target(bool Bar) {
+  bool Foo = true;
+  if (Bar != Foo) {
+(void)0;
+/*[[p-then]]*/
+  } else {
+(void)0;
+/*[[p-else]]*/
+  }
+}
+  )";
+  runDataflow(
+  Code, [](llvm::ArrayRef<
+   std::pair>>
+   Results,
+   ASTContext &ASTCtx) {
+ASSERT_THAT(Results, ElementsAre(Pair("p-else", _), Pair("p-then", _)));
+const Environment &EnvElse = Results[0].second.Env;
+const Environment &EnvThen = Results[1].second.Env;
+
+const ValueDecl *BarDecl = findValueDecl(ASTCtx, "Bar");
+ASSERT_THAT(BarDecl, NotNull());
+
+auto &BarValThen =
+*cast(EnvThen.getValue(*BarDecl, SkipPast::None));
+EXPECT_FALSE(EnvThen.flowConditionImplies(BarValThen));
+
+auto &BarValElse =
+*cast(EnvElse.getValue(*BarDecl, SkipPast::None));
+EXPECT_TRUE(EnvElse.flowConditionImplies(BarValElse));
+  });
+}
+
 } // namespace
Index: clang/lib/Analysis/FlowSensitive/Transfer.cpp
===
--- clang/lib/Analysis/FlowSensitive/Transfer.cpp
+++ clang/lib/Analysis/FlowSensitive/Transfer.cpp
@@ -21,6 +21,7 @@
 #include "clang/AST/Stmt.h"
 #include "clang/AST/StmtVisitor.h"
 #include "clang/Analysis/FlowSensitive/DataflowEnvironment.h"
+#include "clang/Analysis/FlowSensitive/Value.h"
 #include "clang/Basic/OperatorKinds.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/Support/Casting.h"
@@ -37,6 +38,26 @@
   return E;
 }
 
+static BoolValue &evaluateBooleanEquality(const Expr &LHS, const Expr &RHS,
+  Environment &Env) {
+  // Equality of booleans involves implicit integral casts. Ignore these casts
+  // for now and focus on the values associated with the wrapped expressions.
+  // FIXME: Consider changing this once the framework offers better support for
+  // integral casts.
+  const Expr *LHSNorm = LHS.IgnoreCasts();
+  const Expr *RHSNorm = RHS.IgnoreCasts();
+  assert(LHSNorm != nullptr);
+  assert(RHSNorm != nullptr);
+
+  if (auto *LHSValue = dyn_cast_or_null(
+  Env.getValue(*LHSNorm, SkipPast::Reference)))
+if (auto *RHSValue = dyn_cast_or_null(
+Env.getValue(*RHSNorm, SkipPast::Reference)))
+  return Env.makeIff(*LHSValue, *RHSValue);
+
+  return Env.makeAtomicBoolValue();
+}
+
 class TransferVisitor : public ConstStmtVisitor {
 public:
   TransferVisitor(const StmtToEnvMap &StmtToEnv, Environment &Env)
@@ -83,8 +104,16 @@
 Env.setValue(Loc, Env.makeOr(LHSVal, RHSVal));
   break;
 }
+case BO_NE:
+case BO_EQ: {
+  auto &LHSAndRHSValue = evaluateBooleanEquality(*LHS, *RHS, Env);
+  auto &Loc = Env.createStorageLocation(*S);
+  Env.setStorageLocation(*S, Loc);
+  Env.setValue(Loc, S->getOpcode() == BO_EQ ? LHSAndRHSValue
+: Env.makeNot(LHSAndRHSValue));
+  break;
+}
 default:
-  // FIXME: Add support for BO_EQ, BO_NE.
   brea

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

[Gabor Marton via Phabricator via cfe-commits]

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

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.




Comment at: clang/include/clang/CrossTU/CrossTranslationUnit.h:112
 
-// Returns true if the variable or any field of a record variable is const.
-bool containsConst(const VarDecl *VD, const ASTContext &ACtx);
+/// Returns true if it makes sense to import a foreign variable declaration.
+/// For instance, we don't want to import variables that have non-trivial types

steakhal wrote:
> definition
thx, fixed.



Comment at: clang/lib/CrossTU/CrossTranslationUnit.cpp:195
+bool shouldImport(const VarDecl *VD, const ASTContext &ACtx) {
+
   CanQualType CT = ACtx.getCanonicalType(VD->getType());

steakhal wrote:
> extra blank line
Thanks!



Comment at: clang/test/Analysis/Inputs/ctu-other.cpp:128
 SCNest extSCN = {.scn = {.a = 9}};
-SCNest::SCN extSubSCN = {.a = 1};
+extern SCNest::SCN const extSubSCN = {.a = 1};
 struct SCC {

steakhal wrote:
> AFAIK you don't need the `extern` here, since **this is** the definition of 
> that global, not only a declaration.
> Similarly applies to the rest of the cases.
Actually, linkage of `const` variable definitions are internal in C++,  unless 
the `extern` modifier is provided. So, we need it.

If it was a non-const then the definition's linkage would be external.


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] D122774: [clang][extract-api] Add Objective-C Category support

[Zixu Wang via Phabricator via cfe-commits]

zixuw updated this revision to Diff 419474.
zixuw added a comment.

Add missing documentatin for ObjCCategoryRecord.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122774

Files:
  clang/include/clang/ExtractAPI/API.h
  clang/include/clang/ExtractAPI/DeclarationFragments.h
  clang/include/clang/ExtractAPI/Serialization/SymbolGraphSerializer.h
  clang/lib/ExtractAPI/API.cpp
  clang/lib/ExtractAPI/DeclarationFragments.cpp
  clang/lib/ExtractAPI/ExtractAPIConsumer.cpp
  clang/lib/ExtractAPI/Serialization/SymbolGraphSerializer.cpp
  clang/test/ExtractAPI/objc_category.m

Index: clang/test/ExtractAPI/objc_category.m
===
--- /dev/null
+++ clang/test/ExtractAPI/objc_category.m
@@ -0,0 +1,284 @@
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+// RUN: sed -e "s@INPUT_DIR@%/t@g" %t/reference.output.json.in >> \
+// RUN: %t/reference.output.json
+// RUN: %clang -extract-api -x objective-c-header -target arm64-apple-macosx \
+// RUN: %t/input.h -o %t/output.json | FileCheck -allow-empty %s
+
+// Generator version is not consistent across test runs, normalize it.
+// RUN: sed -e "s@\"generator\": \".*\"@\"generator\": \"?\"@g" \
+// RUN: %t/output.json >> %t/output-normalized.json
+// RUN: diff %t/reference.output.json %t/output-normalized.json
+
+// CHECK-NOT: error:
+// CHECK-NOT: warning:
+
+//--- input.h
+@protocol Protocol;
+
+@interface Interface
+@end
+
+@interface Interface (Category) 
+@property int Property;
+- (void)InstanceMethod;
++ (void)ClassMethod;
+@end
+
+//--- reference.output.json.in
+{
+  "metadata": {
+"formatVersion": {
+  "major": 0,
+  "minor": 5,
+  "patch": 3
+},
+"generator": "?"
+  },
+  "module": {
+"name": "",
+"platform": {
+  "architecture": "arm64",
+  "operatingSystem": {
+"minimumVersion": {
+  "major": 11,
+  "minor": 0,
+  "patch": 0
+},
+"name": "macosx"
+  },
+  "vendor": "apple"
+}
+  },
+  "relationhips": [
+{
+  "kind": "memberOf",
+  "source": "c:objc(cs)Interface(im)InstanceMethod",
+  "target": "c:objc(cs)Interface"
+},
+{
+  "kind": "memberOf",
+  "source": "c:objc(cs)Interface(cm)ClassMethod",
+  "target": "c:objc(cs)Interface"
+},
+{
+  "kind": "memberOf",
+  "source": "c:objc(cs)Interface(py)Property",
+  "target": "c:objc(cs)Interface"
+},
+{
+  "kind": "conformsTo",
+  "source": "c:objc(cs)Interface",
+  "target": "c:objc(pl)Protocol"
+}
+  ],
+  "symbols": [
+{
+  "declarationFragments": [
+{
+  "kind": "keyword",
+  "spelling": "@interface"
+},
+{
+  "kind": "text",
+  "spelling": " "
+},
+{
+  "kind": "identifier",
+  "spelling": "Interface"
+}
+  ],
+  "identifier": {
+"interfaceLanguage": "objective-c",
+"precise": "c:objc(cs)Interface"
+  },
+  "kind": {
+"displayName": "Class",
+"identifier": "objective-c.class"
+  },
+  "location": {
+"character": 12,
+"line": 3,
+"uri": "file://INPUT_DIR/input.h"
+  },
+  "names": {
+"subHeading": [
+  {
+"kind": "identifier",
+"spelling": "Interface"
+  }
+],
+"title": "Interface"
+  }
+},
+{
+  "declarationFragments": [
+{
+  "kind": "text",
+  "spelling": "- ("
+},
+{
+  "kind": "typeIdentifier",
+  "preciseIdentifier": "c:v",
+  "spelling": "void"
+},
+{
+  "kind": "text",
+  "spelling": ")"
+},
+{
+  "kind": "identifier",
+  "spelling": "InstanceMethod"
+}
+  ],
+  "identifier": {
+"interfaceLanguage": "objective-c",
+"precise": "c:objc(cs)Interface(im)InstanceMethod"
+  },
+  "kind": {
+"displayName": "Instance Method",
+"identifier": "objective-c.method"
+  },
+  "location": {
+"character": 1,
+"line": 8,
+"uri": "file://INPUT_DIR/input.h"
+  },
+  "names": {
+"subHeading": [
+  {
+"kind": "text",
+"spelling": "- "
+  },
+  {
+"kind": "identifier",
+"spelling": "InstanceMethod"
+  }
+],
+"title": "InstanceMethod"
+  }
+},
+{
+  "declarationFragments": [
+{
+  "kind": "text",
+  "spelling": "+ ("
+},
+{
+  "kind": "typeIdentifier",
+  "preciseIdentifier": "c:v",
+  "spelling": "void"
+},
+{
+  "kind": "text",
+  "spelling": ")"
+},
+{
+  "kind": "identifier",
+  "spel

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

[Gabor Marton via Phabricator via cfe-commits]

martong updated this revision to Diff 419475.
martong marked 3 inline comments as done.
martong added a comment.

- Remove blank line
- Fix comment
- Add new check


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122805

Files:
  clang/include/clang/CrossTU/CrossTranslationUnit.h
  clang/lib/CrossTU/CrossTranslationUnit.cpp
  clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
  clang/test/Analysis/Inputs/ctu-other.cpp
  clang/test/Analysis/Inputs/ctu-other.cpp.externalDefMap.ast-dump.txt
  clang/test/Analysis/ctu-main.cpp
  clang/test/Analysis/func-mapping-test.cpp
  clang/tools/clang-extdef-mapping/ClangExtDefMapGen.cpp

Index: clang/tools/clang-extdef-mapping/ClangExtDefMapGen.cpp
===
--- clang/tools/clang-extdef-mapping/ClangExtDefMapGen.cpp
+++ clang/tools/clang-extdef-mapping/ClangExtDefMapGen.cpp
@@ -64,7 +64,7 @@
   if (const Stmt *Body = FD->getBody())
 addIfInMain(FD, Body->getBeginLoc());
   } else if (const auto *VD = dyn_cast(D)) {
-if (cross_tu::containsConst(VD, Ctx) && VD->hasInit())
+if (cross_tu::shouldImport(VD, Ctx) && VD->hasInit())
   if (const Expr *Init = VD->getInit())
 addIfInMain(VD, Init->getBeginLoc());
   }
Index: clang/test/Analysis/func-mapping-test.cpp
===
--- clang/test/Analysis/func-mapping-test.cpp
+++ clang/test/Analysis/func-mapping-test.cpp
@@ -23,7 +23,7 @@
 struct SF {
   const int a;
 };
-SF sf = {.a = 2};
+extern const SF sf = {.a = 2};
 // CHECK-DAG: c:@sf
 
 struct SStatic {
@@ -39,7 +39,7 @@
   const int a;
   const unsigned int b;
 };
-U u = {.a = 6};
+extern const U u = {.a = 6};
 // CHECK-DAG: c:@u
 
 // No USR can be generated for this.
Index: clang/test/Analysis/ctu-main.cpp
===
--- clang/test/Analysis/ctu-main.cpp
+++ clang/test/Analysis/ctu-main.cpp
@@ -75,6 +75,13 @@
   int a;
 };
 extern const S extS;
+extern S extNonConstS;
+struct NonTrivialS {
+  int a;
+  // User declaring a dtor makes it non-trivial.
+  ~NonTrivialS();
+};
+extern const NonTrivialS extNTS;
 extern const int extHere;
 const int extHere = 6;
 struct A {
@@ -83,9 +90,9 @@
 struct SC {
   const int a;
 };
-extern SC extSC;
+extern const SC extSC;
 struct ST {
-  static struct SC sc;
+  static const struct SC sc;
 };
 struct SCNest {
   struct SCN {
@@ -93,7 +100,7 @@
   } scn;
 };
 extern SCNest extSCN;
-extern SCNest::SCN extSubSCN;
+extern const SCNest::SCN extSubSCN;
 struct SCC {
   SCC(int c);
   const int a;
@@ -103,7 +110,7 @@
   const int a;
   const unsigned int b;
 };
-extern U extU;
+extern const U extU;
 
 void test_virtual_functions(mycls* obj) {
   // The dynamic type is known.
@@ -172,6 +179,9 @@
   clang_analyzer_eval(extInt == 2); // expected-warning{{TRUE}}
   clang_analyzer_eval(intns::extInt == 3); // expected-warning{{TRUE}}
   clang_analyzer_eval(extS.a == 4); // expected-warning{{TRUE}}
+  clang_analyzer_eval(extNonConstS.a == 4); // expected-warning{{UNKNOWN}}
+  // Do not import non-trivial classes' initializers.
+  clang_analyzer_eval(extNTS.a == 4); // expected-warning{{UNKNOWN}}
   clang_analyzer_eval(extHere == 6); // expected-warning{{TRUE}}
   clang_analyzer_eval(A::a == 3); // expected-warning{{TRUE}}
   clang_analyzer_eval(extSC.a == 8); // expected-warning{{TRUE}}
Index: clang/test/Analysis/Inputs/ctu-other.cpp.externalDefMap.ast-dump.txt
===
--- clang/test/Analysis/Inputs/ctu-other.cpp.externalDefMap.ast-dump.txt
+++ clang/test/Analysis/Inputs/ctu-other.cpp.externalDefMap.ast-dump.txt
@@ -18,6 +18,7 @@
 c:@extInt ctu-other.cpp.ast
 c:@N@intns@extInt ctu-other.cpp.ast
 c:@extS ctu-other.cpp.ast
+c:@extNTS ctu-other.cpp.ast
 c:@S@A@a ctu-other.cpp.ast
 c:@extSC ctu-other.cpp.ast
 c:@S@ST@sc ctu-other.cpp.ast
@@ -27,4 +28,4 @@
 c:@extU ctu-other.cpp.ast
 c:@S@TestAnonUnionUSR@Test ctu-other.cpp.ast
 c:@F@testImportOfIncompleteDefaultParmDuringImport#I# ctu-other.cpp.ast
-c:@F@testImportOfDelegateConstructor#I# ctu-other.cpp.ast
\ No newline at end of file
+c:@F@testImportOfDelegateConstructor#I# ctu-other.cpp.ast
Index: clang/test/Analysis/Inputs/ctu-other.cpp
===
--- clang/test/Analysis/Inputs/ctu-other.cpp
+++ clang/test/Analysis/Inputs/ctu-other.cpp
@@ -102,6 +102,11 @@
   int a;
 };
 extern const S extS = {.a = 4};
+struct NonTrivialS {
+  int a;
+  ~NonTrivialS();
+};
+extern const NonTrivialS extNTS = {.a = 4};
 struct A {
   static const int a;
 };
@@ -109,18 +114,18 @@
 struct SC {
   const int a;
 };
-SC extSC = {.a = 8};
+extern const SC extSC = {.a = 8};
 struct ST {
-  static struct SC sc;
+  static const struct SC sc;
 };
-struct SC ST::sc = {.a = 2};
+const struct SC ST::sc = {.a = 2};
 struct SCNest {
   st

[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()) {

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



Comment at: clang/lib/CodeGen/CGBuiltin.cpp:2049
+  static llvm::DenseMap Types;
   if (Types.empty()) {
 Types[Context.CharTy] = "%c";

I wonder if instead of this whole business, this function should just be 
replaced with a 'switch' on the types.



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

When can we hit this case?  Does this mean that any types we dont understand we 
are just treating as void ptrs?  



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.

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.



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

We already know if this is a 'zero' length array, we should probably just not 
emit IR at all in that case.



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

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.



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

Do we want to do anything for the OTHER array types?  



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);

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).


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.

[Amit Kumar Pandey via Phabricator via cfe-commits]

ampandey-AMD added a comment.

Yes, it is missing here because there is some part of the code in 
AMDGPUOpenMP.cpp on which openmp sanitize lit test case is dependent and that 
part of code is dependent on Greg's DLR patch. I will upstream the missing 
source code and lit test case afterwards when the Greg's DLR patch will get 
upstreamed and land.


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

[clang] 4d5b824 - [analyzer] Avoid checking addrspace pointers in cstring checker

[via cfe-commits]

Author: Vince Bridgers
Date: 2022-03-31T17:34:56+02:00
New Revision: 4d5b824e3df21c9cfa7a14ea746c83d0d41dd3ed

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

LOG: [analyzer] Avoid checking addrspace pointers in cstring checker

This change fixes an assert that occurs in the SMT layer when refuting a
finding that uses pointers of two different sizes. This was found in a
downstream build that supports two different pointer sizes, The CString
Checker was attempting to compute an overlap for the 'to' and 'from'
pointers, where the pointers were of different sizes.

In the downstream case where this was found, a specialized memcpy
routine patterned after memcpy_special is used. The analyzer core hits
on this builtin because it matches the 'memcpy' portion of that builtin.
This cannot be duplicated in the upstream test since there are no
specialized builtins that match that pattern, but the case does
reproduce in the accompanying LIT test case. The amdgcn target was used
for this reproducer. See the documentation for AMDGPU address spaces here
https://llvm.org/docs/AMDGPUUsage.html#address-spaces.

The assert seen is:

`*Solver->getSort(LHS) == *Solver->getSort(RHS) && "AST's must have the same 
sort!"'

Ack to steakhal for reviewing the fix, and creating the test case.

Reviewed By: steakhal

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

Added: 
clang/test/Analysis/cstring-addrspace.c

Modified: 
clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp

Removed: 




diff  --git a/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp 
b/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
index 23c10431a5dfb..dd2185270321e 100644
--- a/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
@@ -460,6 +460,11 @@ ProgramStateRef 
CStringChecker::CheckOverlap(CheckerContext &C,
 
   ProgramStateRef stateTrue, stateFalse;
 
+  // Assume 
diff erent address spaces cannot overlap.
+  if (First.Expression->getType()->getPointeeType().getAddressSpace() !=
+  Second.Expression->getType()->getPointeeType().getAddressSpace())
+return state;
+
   // Get the buffer values and make sure they're known locations.
   const LocationContext *LCtx = C.getLocationContext();
   SVal firstVal = state->getSVal(First.Expression, LCtx);

diff  --git a/clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp 
b/clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
index 0bd47ced15a51..65c2564637c18 100644
--- a/clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
+++ b/clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
@@ -715,11 +715,41 @@ static SVal evalBinOpFieldRegionFieldRegion(const 
FieldRegion *LeftFR,
   llvm_unreachable("Fields not found in parent record's definition");
 }
 
+// This is used in debug builds only for now because some downstream users
+// may hit this assert in their subsequent merges.
+// There are still places in the analyzer where equal bitwidth Locs
+// are compared, and need to be found and corrected. Recent previous fixes have
+// addressed the known problems of making NULLs with specific bitwidths
+// for Loc comparisons along with deprecation of APIs for the same purpose.
+//
+static void assertEqualBitWidths(ProgramStateRef State, Loc RhsLoc,
+ Loc LhsLoc) {
+  // Implements a "best effort" check for RhsLoc and LhsLoc bit widths
+  ASTContext &Ctx = State->getStateManager().getContext();
+  uint64_t RhsBitwidth =
+  RhsLoc.getType(Ctx).isNull() ? 0 : Ctx.getTypeSize(RhsLoc.getType(Ctx));
+  uint64_t LhsBitwidth =
+  LhsLoc.getType(Ctx).isNull() ? 0 : Ctx.getTypeSize(LhsLoc.getType(Ctx));
+  if (RhsBitwidth && LhsBitwidth &&
+  (LhsLoc.getSubKind() == RhsLoc.getSubKind())) {
+assert(RhsBitwidth == LhsBitwidth &&
+   "RhsLoc and LhsLoc bitwidth must be same!");
+  }
+}
+
 // FIXME: all this logic will change if/when we have MemRegion::getLocation().
 SVal SimpleSValBuilder::evalBinOpLL(ProgramStateRef state,
   BinaryOperator::Opcode op,
   Loc lhs, Loc rhs,
   QualType resultTy) {
+
+  // Assert that bitwidth of lhs and rhs are the same.
+  // This can happen if two 
diff erent address spaces are used,
+  // and the bitwidths of the address spaces are 
diff erent.
+  // See LIT case clang/test/Analysis/cstring-checker-addressspace.c
+  // FIXME: See comment above in the function assertEqualBitWidths
+  assertEqualBitWidths(state, rhs, lhs);
+
   // Only comparisons and subtractions are valid operations on two pointers.
   // See [C99 6.5.5 through 6.5.14] or [C++0x 5.6 through 5.15].
   // However, if a poin

[PATCH] D118050: [analyzer] Avoid checking addrspace pointers in cstring checker

[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 rG4d5b824e3df2: [analyzer] Avoid checking addrspace pointers 
in cstring checker (authored by vabridgers, committed by einvbri 
).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118050

Files:
  clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
  clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
  clang/test/Analysis/cstring-addrspace.c

Index: clang/test/Analysis/cstring-addrspace.c
===
--- /dev/null
+++ clang/test/Analysis/cstring-addrspace.c
@@ -0,0 +1,64 @@
+// RUN: %clang_analyze_cc1 -triple amdgcn-unknown-unknown \
+// RUN: -analyze -analyzer-checker=core,alpha.unix.cstring \
+// RUN: -analyze -analyzer-checker=debug.ExprInspection \
+// RUN: -analyzer-config crosscheck-with-z3=true -verify %s \
+// RUN: -Wno-incompatible-library-redeclaration
+// REQUIRES: z3
+
+void clang_analyzer_warnIfReached();
+
+// From https://llvm.org/docs/AMDGPUUsage.html#address-spaces,
+// select address space 3 (local), since the pointer size is
+// different than Generic.
+#define DEVICE __attribute__((address_space(3)))
+_Static_assert(sizeof(int *) == 8, "");
+_Static_assert(sizeof(DEVICE int *) == 4, "");
+_Static_assert(sizeof(void *) == 8, "");
+_Static_assert(sizeof(DEVICE void *) == 4, "");
+
+// Copy from host to device memory. Note this is specialized
+// since one of the parameters is assigned an address space such
+// that the sizeof the the pointer is different than the other.
+//
+// Some downstream implementations may have specialized memcpy
+// routines that copy from one address space to another. In cases
+// like that, the address spaces are assumed to not overlap, so the
+// cstring overlap check is not needed. When a static analysis report
+// is generated in as case like this, SMTConv may attempt to create
+// a refutation to Z3 with different bitwidth pointers which lead to
+// a crash. This is not common in directly used upstream compiler builds,
+// but can be seen in specialized downstrean implementations. This case
+// covers those specific instances found and debugged.
+//
+// Since memcpy is a builtin, a specialized builtin instance named like
+// 'memcpy_special' will hit in cstring, triggering this behavior. The
+// best we can do for an upstream test is use the same memcpy function name.
+DEVICE void *memcpy(DEVICE void *dst, const void *src, unsigned long len);
+
+void top1(DEVICE void *dst, void *src, int len) {
+  memcpy(dst, src, len);
+
+  // Create a bugreport for triggering Z3 refutation.
+  clang_analyzer_warnIfReached(); // expected-warning {{REACHABLE}}
+}
+
+void top2(DEVICE int *dst, void *src, int len) {
+  memcpy(dst, src, len);
+
+  // Create a bugreport for triggering Z3 refutation.
+  clang_analyzer_warnIfReached(); // expected-warning {{REACHABLE}}
+}
+
+void top3(DEVICE int *dst, int *src, int len) {
+  memcpy(dst, src, len);
+
+  // Create a bugreport for triggering Z3 refutation.
+  clang_analyzer_warnIfReached(); // expected-warning {{REACHABLE}}
+}
+
+void top4() {
+  memcpy((DEVICE void *)1, (const void *)1, 1);
+
+  // Create a bugreport for triggering Z3 refutation.
+  clang_analyzer_warnIfReached(); // expected-warning {{REACHABLE}}
+}
Index: clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
===
--- clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
+++ clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
@@ -715,11 +715,41 @@
   llvm_unreachable("Fields not found in parent record's definition");
 }
 
+// This is used in debug builds only for now because some downstream users
+// may hit this assert in their subsequent merges.
+// There are still places in the analyzer where equal bitwidth Locs
+// are compared, and need to be found and corrected. Recent previous fixes have
+// addressed the known problems of making NULLs with specific bitwidths
+// for Loc comparisons along with deprecation of APIs for the same purpose.
+//
+static void assertEqualBitWidths(ProgramStateRef State, Loc RhsLoc,
+ Loc LhsLoc) {
+  // Implements a "best effort" check for RhsLoc and LhsLoc bit widths
+  ASTContext &Ctx = State->getStateManager().getContext();
+  uint64_t RhsBitwidth =
+  RhsLoc.getType(Ctx).isNull() ? 0 : Ctx.getTypeSize(RhsLoc.getType(Ctx));
+  uint64_t LhsBitwidth =
+  LhsLoc.getType(Ctx).isNull() ? 0 : Ctx.getTypeSize(LhsLoc.getType(Ctx));
+  if (RhsBitwidth && LhsBitwidth &&
+  (LhsLoc.getSubKind() == RhsLoc.getSubKind())) {
+assert(RhsBitwidth == LhsBitwidth &&
+   "RhsLoc and LhsLoc bitwidth must be same!");
+  }
+}
+
 // FIXME: all this logic will change if/when we have MemRegion::getLocation().
 SVal SimpleSValBuil

[clang] 4dfec37 - [OpenCL] Set MinVersion for sub_group_barrier with memory_scope

[Sven van Haastregt via cfe-commits]

Author: Sven van Haastregt
Date: 2022-03-31T16:41:40+01:00
New Revision: 4dfec37037f5f96db8898b79601c7a1d19177027

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

LOG: [OpenCL] Set MinVersion for sub_group_barrier with memory_scope

The memory_scope enum is not available before OpenCL 2.0, so ensure
the sub_group_barrier overload with a memory_scope argument is
restricted to OpenCL 2.0 and above.  This is already the case in
opencl-c.h.

Fixes the issue revealed by https://reviews.llvm.org/D120254

Reported-by: Harald van Dijk (hvdijk)

Added: 


Modified: 
clang/lib/Sema/OpenCLBuiltins.td

Removed: 




diff  --git a/clang/lib/Sema/OpenCLBuiltins.td 
b/clang/lib/Sema/OpenCLBuiltins.td
index 9c1a39cdb1a56..a57b74430f694 100644
--- a/clang/lib/Sema/OpenCLBuiltins.td
+++ b/clang/lib/Sema/OpenCLBuiltins.td
@@ -1698,7 +1698,9 @@ let Extension = FuncExtKhrSubgroups in {
 // --- Table 28.2.2 ---
 let Extension = FuncExtKhrSubgroups in {
   def : Builtin<"sub_group_barrier", [Void, MemFenceFlags], Attr.Convergent>;
-  def : Builtin<"sub_group_barrier", [Void, MemFenceFlags, MemoryScope], 
Attr.Convergent>;
+  let MinVersion = CL20 in {
+def : Builtin<"sub_group_barrier", [Void, MemFenceFlags, MemoryScope], 
Attr.Convergent>;
+  }
 }
 
 // --- Table 28.2.4 ---



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

[PATCH] D120254: [OpenCL] Align subgroup builtin guards

[Sven van Haastregt via Phabricator via cfe-commits]

svenvh added a comment.

I've submitted the fix in `4dfec37037f5`.

As for testing, unfortunately I couldn't just extend the 
`SemaOpenCL/fdeclare-opencl-builtins.cl` test: the OpenCL 1.2 RUN lines 
explicitly disable the `cl_intel_subgroups` extension (which is the extension 
that brings in `sub_group_barrier` with CL1.2 for the generic `spir` triple).  
Adding a new `RUN` line or new test file for just this case seems a bit of an 
overkill, so I'm tempted to defer this until we have parity between 
`OpenCLBuiltins.td` and `opencl-c.h`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120254

___
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 created this revision.
jhuber6 added reviewers: jdoerfert, JonChesterfield, grokos, ABataev, ronlieb, 
tianshilei1992, saiislam.
Herald added subscribers: asavonic, kerbowa, guansong, yaxunl, mgorny, jvesely.
Herald added a project: All.
jhuber6 requested review of this revision.
Herald added subscribers: openmp-commits, cfe-commits, sstefan1, MaskRay.
Herald added projects: clang, OpenMP.

Previously an opt-in flag `-fopenmp-new-driver` was used to enable the
new offloading driver. After passing tests for a few months it should be
sufficiently mature to flip the switch and make it the default. The new
offloading driver is now enabled if there is OpenMP and OpenMP
offloading present and the new `-fno-openmp-new-driver` is not present.

The new offloading driver has three main benefits over the old method:

- Static library support
- Device-side LTO
- Unified clang driver stages

Depends on D122683 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D122831

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/amdgpu-openmp-system-arch.c
  clang/test/Driver/amdgpu-openmp-toolchain.c
  clang/test/Driver/fat_archive_amdgpu.cpp
  clang/test/Driver/fat_archive_nvptx.cpp
  clang/test/Driver/openmp-offload-gpu.c
  clang/test/Driver/openmp-offload.c
  clang/test/OpenMP/driver-openmp-target.c
  openmp/libomptarget/CMakeLists.txt
  openmp/libomptarget/plugins/CMakeLists.txt
  openmp/libomptarget/plugins/amdgpu/CMakeLists.txt
  openmp/libomptarget/plugins/cuda/CMakeLists.txt
  openmp/libomptarget/test/lit.cfg
  openmp/libomptarget/test/mapping/data_member_ref.cpp
  openmp/libomptarget/test/mapping/declare_mapper_nested_default_mappers.cpp
  openmp/libomptarget/test/mapping/declare_mapper_nested_mappers.cpp
  openmp/libomptarget/test/mapping/lambda_by_value.cpp
  openmp/libomptarget/test/mapping/lambda_mapping.cpp
  openmp/libomptarget/test/mapping/ompx_hold/struct.c
  openmp/libomptarget/test/offloading/bug49021.cpp
  openmp/libomptarget/test/offloading/bug49334.cpp
  openmp/libomptarget/test/offloading/bug49779.cpp
  openmp/libomptarget/test/offloading/bug51781.c
  openmp/libomptarget/test/offloading/host_as_target.c
  openmp/libomptarget/test/offloading/memory_manager.cpp
  openmp/libomptarget/test/offloading/parallel_offloading_map.cpp
  openmp/libomptarget/test/offloading/static_linking.c
  openmp/libomptarget/test/offloading/taskloop_offload_nowait.cpp
  openmp/libomptarget/test/unified_shared_memory/api.c

Index: openmp/libomptarget/test/unified_shared_memory/api.c
===
--- openmp/libomptarget/test/unified_shared_memory/api.c
+++ openmp/libomptarget/test/unified_shared_memory/api.c
@@ -1,11 +1,11 @@
 // RUN: %libomptarget-compile-run-and-check-generic
 // XFAIL: nvptx64-nvidia-cuda
 // XFAIL: nvptx64-nvidia-cuda
-// XFAIL: nvptx64-nvidia-cuda-newDriver
+// XFAIL: nvptx64-nvidia-cuda-oldDriver
 
 // Fails on amdgpu with error: GPU Memory Error
 // XFAIL: amdgcn-amd-amdhsa
-// XFAIL: amdgcn-amd-amdhsa-newDriver
+// XFAIL: amdgcn-amd-amdhsa-oldDriver
 
 #include 
 #include 
Index: openmp/libomptarget/test/offloading/taskloop_offload_nowait.cpp
===
--- openmp/libomptarget/test/offloading/taskloop_offload_nowait.cpp
+++ openmp/libomptarget/test/offloading/taskloop_offload_nowait.cpp
@@ -1,7 +1,7 @@
 // RUN: %libomptarget-compilexx-and-run-generic
 
 // UNSUPPORTED: x86_64-pc-linux-gnu
-// UNSUPPORTED: x86_64-pc-linux-gnu-newDriver
+// UNSUPPORTED: x86_64-pc-linux-gnu-oldDriver
 
 #include 
 #include 
Index: openmp/libomptarget/test/offloading/static_linking.c
===
--- openmp/libomptarget/test/offloading/static_linking.c
+++ openmp/libomptarget/test/offloading/static_linking.c
@@ -2,8 +2,8 @@
 // RUN: llvm-ar rcs %t.a %t.o
 // RUN: %libomptarget-compile-generic %t.a && %libomptarget-run-generic 2>&1 | %fcheck-generic
 
-// REQUIRES: nvptx64-nvidia-cuda-newDriver
-// REQUIRES: amdgcn-amd-amdhsa-newDriver
+// REQUIRES: nvptx64-nvidia-cuda-oldDriver
+// REQUIRES: amdgcn-amd-amdhsa-oldDriver
 
 #ifdef LIBRARY
 int x = 42;
Index: openmp/libomptarget/test/offloading/parallel_offloading_map.cpp
===
--- openmp/libomptarget/test/offloading/parallel_offloading_map.cpp
+++ openmp/libomptarget/test/offloading/parallel_offloading_map.cpp
@@ -1,7 +1,7 @@
 // RUN: %libomptarget-compilexx-run-and-check-generic
 
 // UNSUPPORTED: x86_64-pc-linux-gnu
-// UNSUPPORTED: x86_64-pc-linux-gnu-newDriver
+// UNSUPPORTED: x86_64-pc-linux-gnu-oldDriver
 
 #include 
 #include 
Index: openmp/libomptarget/test/offloading/memory_manager.cpp
===
--- openmp/libomptarget/test/offloading/memory_manager

[PATCH] D122789: [compiler-rt] [scudo] Use -mcrc32 on x86 when available

[Michał Górny 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 rG09b53121c323: [compiler-rt] [scudo] Use -mcrc32 on x86 when 
available (authored by mgorny).
Herald added a subscriber: Sanitizers.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122789

Files:
  compiler-rt/cmake/config-ix.cmake
  compiler-rt/lib/scudo/CMakeLists.txt
  compiler-rt/lib/scudo/scudo_allocator.cpp
  compiler-rt/lib/scudo/scudo_crc32.cpp
  compiler-rt/lib/scudo/scudo_crc32.h
  compiler-rt/lib/scudo/standalone/CMakeLists.txt
  compiler-rt/lib/scudo/standalone/checksum.h
  compiler-rt/lib/scudo/standalone/chunk.h
  compiler-rt/lib/scudo/standalone/crc32_hw.cpp

Index: compiler-rt/lib/scudo/standalone/crc32_hw.cpp
===
--- compiler-rt/lib/scudo/standalone/crc32_hw.cpp
+++ compiler-rt/lib/scudo/standalone/crc32_hw.cpp
@@ -10,10 +10,10 @@
 
 namespace scudo {
 
-#if defined(__SSE4_2__) || defined(__ARM_FEATURE_CRC32)
+#if defined(__CRC32__) || defined(__SSE4_2__) || defined(__ARM_FEATURE_CRC32)
 u32 computeHardwareCRC32(u32 Crc, uptr Data) {
   return static_cast(CRC32_INTRINSIC(Crc, Data));
 }
-#endif // defined(__SSE4_2__) || defined(__ARM_FEATURE_CRC32)
+#endif // defined(__CRC32__) || defined(__SSE4_2__) || defined(__ARM_FEATURE_CRC32)
 
 } // namespace scudo
Index: compiler-rt/lib/scudo/standalone/chunk.h
===
--- compiler-rt/lib/scudo/standalone/chunk.h
+++ compiler-rt/lib/scudo/standalone/chunk.h
@@ -25,7 +25,7 @@
   // as opposed to only for crc32_hw.cpp. This means that other hardware
   // specific instructions were likely emitted at other places, and as a result
   // there is no reason to not use it here.
-#if defined(__SSE4_2__) || defined(__ARM_FEATURE_CRC32)
+#if defined(__CRC32__) || defined(__SSE4_2__) || defined(__ARM_FEATURE_CRC32)
   u32 Crc = static_cast(CRC32_INTRINSIC(Seed, Value));
   for (uptr I = 0; I < ArraySize; I++)
 Crc = static_cast(CRC32_INTRINSIC(Crc, Array[I]));
@@ -42,7 +42,7 @@
   Checksum = computeBSDChecksum(Checksum, Array[I]);
 return Checksum;
   }
-#endif // defined(__SSE4_2__) || defined(__ARM_FEATURE_CRC32)
+#endif // defined(__CRC32__) || defined(__SSE4_2__) || defined(__ARM_FEATURE_CRC32)
 }
 
 namespace Chunk {
Index: compiler-rt/lib/scudo/standalone/checksum.h
===
--- compiler-rt/lib/scudo/standalone/checksum.h
+++ compiler-rt/lib/scudo/standalone/checksum.h
@@ -12,12 +12,13 @@
 #include "internal_defs.h"
 
 // Hardware CRC32 is supported at compilation via the following:
-// - for i386 & x86_64: -msse4.2
+// - for i386 & x86_64: -mcrc32 (earlier: -msse4.2)
 // - for ARM & AArch64: -march=armv8-a+crc or -mcrc
 // An additional check must be performed at runtime as well to make sure the
 // emitted instructions are valid on the target host.
 
-#ifdef __SSE4_2__
+#if defined(__CRC32__) || defined(__SSE4_2__)
+// NB: clang has  but GCC does not
 #include 
 #define CRC32_INTRINSIC FIRST_32_SECOND_64(_mm_crc32_u32, _mm_crc32_u64)
 #endif
Index: compiler-rt/lib/scudo/standalone/CMakeLists.txt
===
--- compiler-rt/lib/scudo/standalone/CMakeLists.txt
+++ compiler-rt/lib/scudo/standalone/CMakeLists.txt
@@ -97,8 +97,11 @@
   string_utils.cpp
   )
 
-# Enable the SSE 4.2 instruction set for crc32_hw.cpp, if available.
-if (COMPILER_RT_HAS_MSSE4_2_FLAG)
+# Enable the necessary instruction set for scudo_crc32.cpp, if available.
+# Newer compiler versions use -mcrc32 rather than -msse4.2.
+if (COMPILER_RT_HAS_MCRC32_FLAG)
+  set_source_files_properties(crc32_hw.cpp PROPERTIES COMPILE_FLAGS -mcrc32)
+elseif (COMPILER_RT_HAS_MSSE4_2_FLAG)
   set_source_files_properties(crc32_hw.cpp PROPERTIES COMPILE_FLAGS -msse4.2)
 endif()
 
Index: compiler-rt/lib/scudo/scudo_crc32.h
===
--- compiler-rt/lib/scudo/scudo_crc32.h
+++ compiler-rt/lib/scudo/scudo_crc32.h
@@ -16,13 +16,14 @@
 #include "sanitizer_common/sanitizer_internal_defs.h"
 
 // Hardware CRC32 is supported at compilation via the following:
-// - for i386 & x86_64: -msse4.2
+// - for i386 & x86_64: -mcrc32 (earlier: -msse4.2)
 // - for ARM & AArch64: -march=armv8-a+crc or -mcrc
 // An additional check must be performed at runtime as well to make sure the
 // emitted instructions are valid on the target host.
 
-#if defined(__SSE4_2__) || defined(__ARM_FEATURE_CRC32)
-# ifdef __SSE4_2__
+#if defined(__CRC32__) || defined(__SSE4_2__) || defined(__ARM_FEATURE_CRC32)
+# if defined(__CRC32__) || defined(__SSE4_2__)
+// NB: clang has  but GCC does not
 #  include 
 #  define CRC32_INTRINSIC FIRST_32_SECOND_64(_mm_crc32_u32, _mm_crc32_u64)
 # endif
@@ -30,7 +31,7 @@
 # 

[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);

This line here accesses NeededSSE while uninitialized.  Do you  need to 
initalize it to something on 3963?


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] D122683: [OpenMP] Use new offloading binary when embedding offloading images

[Joseph Huber via Phabricator via cfe-commits]

jhuber6 updated this revision to Diff 419490.
jhuber6 added a comment.

Fixing unrelated formatting changes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122683

Files:
  clang/include/clang/Basic/CodeGenOptions.h
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/openmp-offload-gpu.c
  clang/test/Frontend/embed-object.c
  clang/test/Frontend/embed-object.ll
  clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp
  llvm/include/llvm/Object/OffloadBinary.h
  llvm/include/llvm/Transforms/Utils/ModuleUtils.h
  llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp
  llvm/lib/Transforms/Utils/ModuleUtils.cpp

Index: llvm/lib/Transforms/Utils/ModuleUtils.cpp
===
--- llvm/lib/Transforms/Utils/ModuleUtils.cpp
+++ llvm/lib/Transforms/Utils/ModuleUtils.cpp
@@ -265,15 +265,15 @@
 }
 
 void llvm::embedBufferInModule(Module &M, MemoryBufferRef Buf,
-   StringRef SectionName) {
-  // Embed the buffer into the module.
+   StringRef SectionName, Align Alignment) {
+  // Embed the memory buffer into the module.
   Constant *ModuleConstant = ConstantDataArray::get(
   M.getContext(), makeArrayRef(Buf.getBufferStart(), Buf.getBufferSize()));
   GlobalVariable *GV = new GlobalVariable(
-  M, ModuleConstant->getType(), true, GlobalValue::ExternalLinkage,
-  ModuleConstant, SectionName.drop_front());
+  M, ModuleConstant->getType(), true, GlobalValue::PrivateLinkage,
+  ModuleConstant, "llvm.embedded.object");
   GV->setSection(SectionName);
-  GV->setVisibility(GlobalValue::HiddenVisibility);
+  GV->setAlignment(Alignment);
 
   appendToCompilerUsed(M, GV);
 }
Index: llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp
===
--- llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp
+++ llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp
@@ -447,7 +447,7 @@
   Name == getInstrProfSectionName(IPSK_covfun, Triple::ELF,
   /*AddSegmentInfo=*/false) ||
   Name == ".llvmbc" || Name == ".llvmcmd" ||
-  Name.startswith(".llvm.offloading."))
+  Name.startswith(".llvm.offloading"))
 return SectionKind::getMetadata();
 
   if (Name.empty() || Name[0] != '.') return K;
Index: llvm/include/llvm/Transforms/Utils/ModuleUtils.h
===
--- llvm/include/llvm/Transforms/Utils/ModuleUtils.h
+++ llvm/include/llvm/Transforms/Utils/ModuleUtils.h
@@ -14,6 +14,7 @@
 #define LLVM_TRANSFORMS_UTILS_MODULEUTILS_H
 
 #include "llvm/ADT/StringRef.h"
+#include "llvm/Support/Alignment.h"
 #include "llvm/Support/MemoryBufferRef.h"
 #include  // for std::pair
 
@@ -109,7 +110,8 @@
 
 /// Embed the memory buffer \p Buf into the module \p M as a global using the
 /// specified section name.
-void embedBufferInModule(Module &M, MemoryBufferRef Buf, StringRef SectionName);
+void embedBufferInModule(Module &M, MemoryBufferRef Buf, StringRef SectionName,
+ Align Alignment = Align(1));
 
 class CallInst;
 namespace VFABI {
Index: llvm/include/llvm/Object/OffloadBinary.h
===
--- llvm/include/llvm/Object/OffloadBinary.h
+++ llvm/include/llvm/Object/OffloadBinary.h
@@ -71,6 +71,7 @@
 
   static uint64_t getAlignment() { return alignof(Header); }
 
+  uint32_t getVersion() const { return TheHeader->Version; }
   uint16_t getImageKind() const { return TheEntry->ImageKind; }
   uint16_t getOffloadKind() const { return TheEntry->OffloadKind; }
   uint32_t getFlags() const { return TheEntry->Flags; }
Index: clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp
===
--- clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp
+++ clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp
@@ -15,6 +15,7 @@
 //===-===//
 
 #include "OffloadWrapper.h"
+
 #include "clang/Basic/Version.h"
 #include "llvm/BinaryFormat/Magic.h"
 #include "llvm/Bitcode/BitcodeWriter.h"
@@ -29,6 +30,7 @@
 #include "llvm/Object/ArchiveWriter.h"
 #include "llvm/Object/Binary.h"
 #include "llvm/Object/ObjectFile.h"
+#include "llvm/Object/OffloadBinary.h"
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/Errc.h"
 #include "llvm/Support/FileOutputBuffer.h"
@@ -47,6 +49,7 @@
 #include "llvm/Target/TargetMachine.h"
 
 using namespace llvm;
+using namespace clang;
 using namespace llvm::object;
 
 static cl::opt Help("h", cl::desc("Alias for -help"), cl::Hidden);
@@ -146,9 +149,10 @@
 static codegen::RegisterCodeGenFlags CodeGenFlags;
 
 /// Magic section string that marks the existence of offloading data. The
-/// section string will be formatted as 

[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 
&AST) {
  &AST.getTokens());
 }
 
-ReferencedFiles findReferencedFiles(
-const ReferencedLocations &Locs, const SourceManager &SM,
-llvm::function_ref HeaderResponsible,
-llvm::function_ref(FileID)> UmbrellaHeader) {
+ReferencedFiles
+findReferencedFiles(const ReferencedLocations &Locs, const SourceManager &SM,
+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 &Symbol : Locs.Stdlib)
 for (const auto &Header : 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 &Locs,
 const IncludeStructure &Includes,
-const CanonicalIncludes &CanonIncludes,
 const SourceManager &SM) {
-  return findReferencedFiles(
-  Locs, SM,
-  [&SM, &Includes](FileID ID) {
-return headerResponsible(ID, SM, Includes);
-  },
-  [&SM, &CanonIncludes](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, [&SM, &Includes](FileID ID) {
+return headerResponsible(ID, SM, Includes);
+  });
 }
 
 std::vector
 getUnused(ParsedAST &AST,
-  const llvm::DenseSet &ReferencedFiles,
-  const llvm::StringSet<> &ReferencedPublicHeaders) {
+  const llvm::DenseSet &ReferencedFiles) {
   trace::Span Tracer("IncludeCleaner::getUnused");
   std::vector Unused;
   for (const Inclusion &MFI : 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 &AST) {
   const auto &SM = AST.getSourceManager();
 
   auto Refs = findReferencedLocations(AST);
-  auto ReferencedFiles =
-  findReferencedFiles(Refs, AST.getIncludeStructure(),
-  AST.getCanonicalIncludes(), AST.getSourceManager());
+  auto ReferencedFileIDs = findReferencedFiles(Refs, AST.

[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 &FI) 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

[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

[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] 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 &R : 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 &R) {
@@ -405,6 +432,11 @@
   OptsWithMarshalling.push_back(&R);
   }
   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] 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] 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] 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] 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] 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 &F, MachineFrameInfo &FrameInfo) {
+  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 &N = Existing->getOperand(0)) {
+if (cast(N.get())->getString() == MetadataName) {
+  if (auto &Op = 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 (F.hasFnAttrib

[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] 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 &ASTCtx) {
+ASSERT_THAT(Results, ElementsAre(Pair("p", _)));
+const Environment &Env = 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 &Base :
+ 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 &FooVal = *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 &ASTCtx) {
+ASSERT_THAT(Results, ElementsAre(Pair("p", _)));
+const Environment &Env = 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 &Base :
+ FooDecl->getType()->getAsCXXRecordDecl()->ba

[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] 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] 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 &ASTCtx) {
+ASSERT_THAT(Results, ElementsAre(Pair("p", _)));
+const Environment &Env = 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 &Base :
+ 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 &FooVal = *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 &ASTCtx) {
+ASSERT_THAT(Results, ElementsAre(Pair("p", _)));
+const Environment &Env = Resu

[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

[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 &AL);
   BTFDeclTagAttr *mergeBTFDeclTagAttr(Decl *D, const BTFDeclTagAttr &AL);
+  HLSLNumThreadsAttr *mergeHLSLNumThreadsAttr(Decl *D,
+  const AttributeCommonInfo &AL,
+  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 &S, 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 &S, Decl *D, 
const ParsedAttr &AL) {
 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 
&AL,
+  int X, int Y, int Z) {
+  if (HLSLNumThreadsAttr *NT = D->getAttr()) {

[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

[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] 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] 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] 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] 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] 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] 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] 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] 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 executions.

[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&R 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 4cc

[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