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

[Alexander Richardson via Phabricator via cfe-commits]

arichardson added a comment.

In D135171#4657080 , @brad wrote:

> You can close this.

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


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135171

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

[PATCH] D155456: [RISCV] Support -m[no-]strict-align options

[Alexander Richardson via Phabricator via cfe-commits]

arichardson added inline comments.



Comment at: clang/test/Driver/riscv-features.c:41
+// DEFAULT: "-target-feature" "-unaligned-scalar-mem"
+// DEFAULT-NOT: "-target-feature" "+unaligned-scalar-mem"
+

This looks a bit fragile, can we just check all -target-feature flags instead 
and add --implicit-check-not='-target-feature" to Filecheck?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155456

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

[PATCH] D153989: [compiler-rt] Move crt into builtins

[Alexander Richardson via Phabricator via cfe-commits]

arichardson added a comment.

In D153989#4492342 , @phosek wrote:

> In D153989#4492229 , @arichardson 
> wrote:
>
>> I think reduplication of build logic makes sense but these are conceptually 
>> separate things. The compiler rt builtins are needed in many cases where the 
>> C start-up code isn't (e.g. baremetal OS code). Would moving it to a 
>> subdirectory of builtins/ work too? That way it's clearer which source files 
>> and tests are part of each component?
>
> That's certainly possible, although I'd also argue that `crtbegin.c` and 
> `crtend.c` isn't the only code in builtins directory that fits into this 
> category (that is not needed in environments like baremetal, requiring C 
> library): for example `atomic*.c`, `os_version_check.c`, `emutls.c`, 
> `enable_execute_stack.c`, `eprintf.c`, `gcc_personality_v0.c`, 
> `clear_cache.c` and probably some more that I missed. If we're going to 
> reorganize things, we should probably do it more holistically.

That is a good point and it would be nice to have more subdirectories (e.g. 
atomic should end up in libatomic.a). I was just suggesting subdirectories for 
this change since we already have an existing hierarchy that could be retained. 
Splitting things out later is slightly more involved.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153989

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

[PATCH] D153989: [compiler-rt] Move crt into builtins

[Alexander Richardson via Phabricator via cfe-commits]

arichardson added a comment.

I think reduplication of build logic makes sense but these are conceptually 
separate things. The compiler rt builtins are needed in many cases where the C 
start-up code isn't (e.g. baremetal OS code). Would moving it to a subdirectory 
of builtins/ work too? That way it's clearer which source files and tests are 
part of each component?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153989

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

[PATCH] D152285: Add support for the NO_COLOR environment variable

[Alexander Richardson via Phabricator via cfe-commits]

arichardson added a comment.



>>> [...] I don't know what compelling use case there is for forcing colors 
>>> *on*, [...] until we know why users need to force-enable colors.
>>
>> The reason is that adding `-fcolor-diagnostics` to the command line is not 
>> always feasible, e.g. most build systems would rebuild everything in that 
>> case. Relying on tty detection also doesn't work as many build tools buffer 
>> the output (and some CIs, too).
>
> Ah, that's an interesting use case, thank you for mentioning it! I'm not 
> certain that's super compelling to me; color diagnostics are on by default, 
> so if someone forcibly disables them in the build system (which seems to not 
> really happen too much in practice: 
> https://sourcegraph.com/search?q=context:global+-fno-color-diagnostics+file:.*Makefile.*=standard=0=repo),
>  that's likely done for a reason.

I had the same use case quite a few years ago: I wanted to have Jenkins output 
coloured diagnostics in the logs to make them slightly easier to read. since 
the output goes to a file rather than a terminal it defaults to no colours and 
there were too many different build systems involved that adding compiler flags 
was not feasible. In the end I added a local hack to clang to default to 
colours being on if a given environment var is set 
(https://github.com/CTSRD-CHERI/llvm-project/commit/a427ffe13c19276ae94cc757632439fda08d9dc6).
 I'd be happy if we ended up supporting CLICOLOR_FORCE=1 as a way of ignoring 
the isatty check but colours in Jenkins without modifying build systems is just 
a nice to have feature rather than something super useful and there are better 
ways of making the warnings more visible (e.g. the plugin that scans the build 
logs and aggregates results).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152285

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

[PATCH] D142934: clang: Use ptrmask for pointer alignment

[Alexander Richardson via Phabricator via cfe-commits]

arichardson added a comment.

The `__builtin_align_{up,down}` code generation could also make use of this 
IIRC. I think the reason I didn't do this initially was concerns about ptrmask 
not being optimized as well.


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

https://reviews.llvm.org/D142934

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

[PATCH] D144341: [Driver][FreeBSD] Correct driver behavior if a triple is provided without a version

[Alexander Richardson via Phabricator via cfe-commits]

arichardson added a comment.

LGTM, but I'll let @dim and @emaste decide if FreeBSD 8 code is still needed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144341

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

[PATCH] D142048: [Phabricator] Fix __ptr32 arguments passed to builtins

[Alexander Richardson via Phabricator via cfe-commits]

arichardson added a comment.

It sounds to me that the logic from b919c7d should have been restricted to 
actual LLVM intrinsics that are overloaded on the pointer args ( e.g. 
llvm.memcpy which was the test in that commit). Builtins that are just library 
calls with known semantics probably need the cast even for opencl?




Comment at: clang/test/CodeGen/address-space-ptr32.c:54
+  // CHECK-NEXT:   ret i64 %call
+   return strlen ( s );
+}

Maybe use `__builtin_strlen` here instead to avoid the declaration?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142048

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

[PATCH] D139114: [Clang][Sema] Enabled implicit conversion warning for CompoundAssignment operator.

[Alexander Richardson via Phabricator via cfe-commits]

arichardson added inline comments.



Comment at: clang/lib/Sema/SemaChecking.cpp:13402
+  // intergral operands.
+  if (E->getLHS()->getType()->isIntegerType() &&
+  E->getRHS()->getType()->isIntegerType() && !E->isShiftAssignOp())

Why is this restricted to integers? It looks like `CheckImplicitConversion` 
also handles other types? Does calling it unconditionally cause any issues?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139114

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

[PATCH] D140218: [update_cc_test_checks] Default to --function-signature for new tests

[Alexander Richardson via Phabricator via cfe-commits]

arichardson added a comment.

In D140218#4001835 , @jdoerfert wrote:

> I don't understand why we would remove the flag for _cc_. I feel all these 
> patches are going exactly in the opposite direction I would think one would 
> go.

The flag is not being removed it's just inside common.py now (see D140212 
) and defaults to on for new tests without 
the auto-generated note.




Comment at: 
clang/test/utils/update_cc_test_checks/Inputs/basic-cplusplus.cpp.expected:3
 // Basic C++ test for update_cc_test_checks
 // RUN: %clang_cc1 -no-opaque-pointers -triple=x86_64-unknown-linux-gnu 
-emit-llvm -o - %s | FileCheck %s
 

arsenm wrote:
> Why do we still have generated tests using -no-opaque-pointers? Surely that's 
> the lowest hanging fruit?
Good question, I can update this as a separate NFC change on Monday.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140218

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

[PATCH] D140218: [update_cc_test_checks] Default to --function-signature for new tests

[Alexander Richardson via Phabricator via cfe-commits]

arichardson created this revision.
arichardson added reviewers: nikic, lebedev.ri, jdoerfert, spatel, sebastian-ne.
Herald added a project: All.
arichardson requested review of this revision.
Herald added subscribers: llvm-commits, cfe-commits, sstefan1.
Herald added projects: clang, LLVM.

This is the same as D140212  just for the 
clang update script. Quite a
bit of churn in the internal tests since they used inputs without the
`autogenerated by` args.

Depends on D140212 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D140218

Files:
  clang/test/utils/update_cc_test_checks/Inputs/basic-cplusplus.cpp.expected
  
clang/test/utils/update_cc_test_checks/Inputs/check-attributes.cpp.funcattrs.expected
  
clang/test/utils/update_cc_test_checks/Inputs/check-attributes.cpp.plain.expected
  clang/test/utils/update_cc_test_checks/Inputs/def-and-decl.c.expected
  clang/test/utils/update_cc_test_checks/Inputs/exec-all-runlines.c
  
clang/test/utils/update_cc_test_checks/Inputs/explicit-template-instantiation.cpp
  clang/test/utils/update_cc_test_checks/Inputs/generated-funcs-regex.c.expected
  
clang/test/utils/update_cc_test_checks/Inputs/generated-funcs.c.generated.expected
  
clang/test/utils/update_cc_test_checks/Inputs/generated-funcs.c.no-generated.expected
  
clang/test/utils/update_cc_test_checks/Inputs/global-hex-value-regex.c.expected
  clang/test/utils/update_cc_test_checks/Inputs/global-value-regex.c.expected
  clang/test/utils/update_cc_test_checks/Inputs/ifdef.c.expected
  clang/test/utils/update_cc_test_checks/Inputs/mangled_names.c
  clang/test/utils/update_cc_test_checks/Inputs/no-funcsig-for-existing-note.c
  
clang/test/utils/update_cc_test_checks/Inputs/no-funcsig-for-existing-note.c.expected-with-flag
  
clang/test/utils/update_cc_test_checks/Inputs/no-funcsig-for-existing-note.c.expected-with-negative-flag
  
clang/test/utils/update_cc_test_checks/Inputs/no-funcsig-for-existing-note.c.expected-with-unrelated-flag
  clang/test/utils/update_cc_test_checks/Inputs/on_the_fly_arg_change.c.expected
  
clang/test/utils/update_cc_test_checks/Inputs/prefix-never-matches.cpp.expected
  
clang/test/utils/update_cc_test_checks/Inputs/replace-value-regex-across-runs.c.expected
  
clang/test/utils/update_cc_test_checks/Inputs/resolve-tmp-conflict.cpp.expected
  clang/test/utils/update_cc_test_checks/check-globals.test
  clang/test/utils/update_cc_test_checks/mangled_names.test
  clang/test/utils/update_cc_test_checks/no-funcsig-for-existing-note.test
  llvm/utils/update_cc_test_checks.py

Index: llvm/utils/update_cc_test_checks.py
===
--- llvm/utils/update_cc_test_checks.py
+++ llvm/utils/update_cc_test_checks.py
@@ -165,14 +165,12 @@
   parser.add_argument(
   '--x86_extra_scrub', action='store_true',
   help='Use more regex for x86 matching to reduce diffs between various subtargets')
-  parser.add_argument('--function-signature', action='store_true',
-  help='Keep function signature information around for the check line')
   parser.add_argument('--check-attributes', action='store_true',
   help='Check "Function Attributes" for functions')
   parser.add_argument('--check-globals', action='store_true',
   help='Check global entries (global variables, metadata, attribute sets, ...) for functions')
   parser.add_argument('tests', nargs='+')
-  args = common.parse_commandline_args(parser)
+  args = common.parse_commandline_args(parser, add_funcsig_flag=True)
   infer_dependent_args(args)
 
   if not find_executable(args.clang):
Index: clang/test/utils/update_cc_test_checks/no-funcsig-for-existing-note.test
===
--- /dev/null
+++ clang/test/utils/update_cc_test_checks/no-funcsig-for-existing-note.test
@@ -0,0 +1,31 @@
+## Check that we do not add function signature matches for tests that already
+## have an autogeneration note since that would cause lots of test churn.
+# RUN: cp -f %S/Inputs/no-funcsig-for-existing-note.c %t.c && %update_cc_test_checks %t.c
+# RUN: diff -u %t.c %S/Inputs/no-funcsig-for-existing-note.c
+## Check that running the script again does not change the result:
+# RUN: %update_cc_test_checks %t.c
+# RUN: diff -u %t.c %S/Inputs/no-funcsig-for-existing-note.c
+## Adding an explicit --function-signature flag should add the lines though:
+# RUN: %update_cc_test_checks %t.c --function-signature
+# RUN: diff -u %t.c %S/Inputs/no-funcsig-for-existing-note.c.expected-with-flag
+## Running the script without arguments again should keep the function signature
+# RUN: %update_cc_test_checks %t.c
+# RUN: diff -u %t.c %S/Inputs/no-funcsig-for-existing-note.c.expected-with-flag
+
+## Check that an unrelated flag in UTC_ARGS does not implicitly enable --function-signature
+# RUN: cp -f 

[PATCH] D139006: [UpdateTestChecks] Match define for labels

[Alexander Richardson via Phabricator via cfe-commits]

arichardson added a comment.

I like the idea of defaulting to --function-signature for new tests a lot, so 
I've implemented that in D140212 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139006

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

[PATCH] D138316: [ASTContext] Avoid duplicating address space map. NFCI

[Alexander Richardson 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 rGc4c23527d6c9: [ASTContext] Avoid duplicating address space 
map. NFCI (authored by arichardson).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138316

Files:
  clang/include/clang/AST/ASTContext.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/Basic/TargetInfo.cpp

Index: clang/lib/Basic/TargetInfo.cpp
===
--- clang/lib/Basic/TargetInfo.cpp
+++ clang/lib/Basic/TargetInfo.cpp
@@ -24,6 +24,30 @@
 using namespace clang;
 
 static const LangASMap DefaultAddrSpaceMap = {0};
+// The fake address space map must have a distinct entry for each
+// language-specific address space.
+static const LangASMap FakeAddrSpaceMap = {
+0,  // Default
+1,  // opencl_global
+3,  // opencl_local
+2,  // opencl_constant
+0,  // opencl_private
+4,  // opencl_generic
+5,  // opencl_global_device
+6,  // opencl_global_host
+7,  // cuda_device
+8,  // cuda_constant
+9,  // cuda_shared
+1,  // sycl_global
+5,  // sycl_global_device
+6,  // sycl_global_host
+3,  // sycl_local
+0,  // sycl_private
+10, // ptr32_sptr
+11, // ptr32_uptr
+12, // ptr64
+13, // hlsl_groupshared
+};
 
 // TargetInfo Constructor.
 TargetInfo::TargetInfo(const llvm::Triple ) : Triple(T) {
@@ -487,6 +511,9 @@
 
   if (Opts.MaxBitIntWidth)
 MaxBitIntWidth = Opts.MaxBitIntWidth;
+
+  if (Opts.FakeAddressSpaceMap)
+AddrSpaceMap = 
 }
 
 bool TargetInfo::initFeatureMap(
Index: clang/lib/AST/ASTContext.cpp
===
--- clang/lib/AST/ASTContext.cpp
+++ clang/lib/AST/ASTContext.cpp
@@ -930,39 +930,6 @@
   return *ParentMapCtx.get();
 }
 
-static const LangASMap *getAddressSpaceMap(const TargetInfo ,
-   const LangOptions ) {
-  if (LOpts.FakeAddressSpaceMap) {
-// The fake address space map must have a distinct entry for each
-// language-specific address space.
-static const unsigned FakeAddrSpaceMap[] = {
-0,  // Default
-1,  // opencl_global
-3,  // opencl_local
-2,  // opencl_constant
-0,  // opencl_private
-4,  // opencl_generic
-5,  // opencl_global_device
-6,  // opencl_global_host
-7,  // cuda_device
-8,  // cuda_constant
-9,  // cuda_shared
-1,  // sycl_global
-5,  // sycl_global_device
-6,  // sycl_global_host
-3,  // sycl_local
-0,  // sycl_private
-10, // ptr32_sptr
-11, // ptr32_uptr
-12, // ptr64
-13, // hlsl_groupshared
-};
-return 
-  } else {
-return ();
-  }
-}
-
 static bool isAddrSpaceMapManglingEnabled(const TargetInfo ,
   const LangOptions ) {
   switch (LangOpts.getAddressSpaceMapMangling()) {
@@ -1293,7 +1260,6 @@
   this->AuxTarget = AuxTarget;
 
   ABI.reset(createCXXABI(Target));
-  AddrSpaceMap = getAddressSpaceMap(Target, LangOpts);
   AddrSpaceMapMangling = isAddrSpaceMapManglingEnabled(Target, LangOpts);
 
   // C99 6.2.5p19.
@@ -12244,10 +12210,7 @@
 }
 
 unsigned ASTContext::getTargetAddressSpace(LangAS AS) const {
-  if (isTargetAddressSpace(AS))
-return toTargetAddressSpace(AS);
-  else
-return (*AddrSpaceMap)[(unsigned)AS];
+  return getTargetInfo().getTargetAddressSpace(AS);
 }
 
 bool ASTContext::hasSameExpr(const Expr *X, const Expr *Y) const {
Index: clang/include/clang/AST/ASTContext.h
===
--- clang/include/clang/AST/ASTContext.h
+++ clang/include/clang/AST/ASTContext.h
@@ -613,9 +613,6 @@
   std::unique_ptr ABI;
   CXXABI *createCXXABI(const TargetInfo );
 
-  /// The logical -> physical address space map.
-  const LangASMap *AddrSpaceMap = nullptr;
-
   /// Address space map mangling must be used with language specific
   /// address spaces (e.g. OpenCL/CUDA)
   bool AddrSpaceMapMangling;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D138316: [ASTContext] Avoid duplicating address space map. NFCI

[Alexander Richardson via Phabricator via cfe-commits]

arichardson added a comment.

As this is a rather simple cleanup, I'll go ahead and merge this next week 
unless there is any objection until then.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138316

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

[PATCH] D138316: [ASTContext] Avoid duplicating address space map. NFCI

[Alexander Richardson via Phabricator via cfe-commits]

arichardson updated this revision to Diff 481664.
arichardson added a comment.

rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138316

Files:
  clang/include/clang/AST/ASTContext.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/Basic/TargetInfo.cpp

Index: clang/lib/Basic/TargetInfo.cpp
===
--- clang/lib/Basic/TargetInfo.cpp
+++ clang/lib/Basic/TargetInfo.cpp
@@ -24,6 +24,30 @@
 using namespace clang;
 
 static const LangASMap DefaultAddrSpaceMap = {0};
+// The fake address space map must have a distinct entry for each
+// language-specific address space.
+static const LangASMap FakeAddrSpaceMap = {
+0,  // Default
+1,  // opencl_global
+3,  // opencl_local
+2,  // opencl_constant
+0,  // opencl_private
+4,  // opencl_generic
+5,  // opencl_global_device
+6,  // opencl_global_host
+7,  // cuda_device
+8,  // cuda_constant
+9,  // cuda_shared
+1,  // sycl_global
+5,  // sycl_global_device
+6,  // sycl_global_host
+3,  // sycl_local
+0,  // sycl_private
+10, // ptr32_sptr
+11, // ptr32_uptr
+12, // ptr64
+13, // hlsl_groupshared
+};
 
 // TargetInfo Constructor.
 TargetInfo::TargetInfo(const llvm::Triple ) : Triple(T) {
@@ -487,6 +511,9 @@
 
   if (Opts.MaxBitIntWidth)
 MaxBitIntWidth = Opts.MaxBitIntWidth;
+
+  if (Opts.FakeAddressSpaceMap)
+AddrSpaceMap = 
 }
 
 bool TargetInfo::initFeatureMap(
Index: clang/lib/AST/ASTContext.cpp
===
--- clang/lib/AST/ASTContext.cpp
+++ clang/lib/AST/ASTContext.cpp
@@ -930,39 +930,6 @@
   return *ParentMapCtx.get();
 }
 
-static const LangASMap *getAddressSpaceMap(const TargetInfo ,
-   const LangOptions ) {
-  if (LOpts.FakeAddressSpaceMap) {
-// The fake address space map must have a distinct entry for each
-// language-specific address space.
-static const unsigned FakeAddrSpaceMap[] = {
-0,  // Default
-1,  // opencl_global
-3,  // opencl_local
-2,  // opencl_constant
-0,  // opencl_private
-4,  // opencl_generic
-5,  // opencl_global_device
-6,  // opencl_global_host
-7,  // cuda_device
-8,  // cuda_constant
-9,  // cuda_shared
-1,  // sycl_global
-5,  // sycl_global_device
-6,  // sycl_global_host
-3,  // sycl_local
-0,  // sycl_private
-10, // ptr32_sptr
-11, // ptr32_uptr
-12, // ptr64
-13, // hlsl_groupshared
-};
-return 
-  } else {
-return ();
-  }
-}
-
 static bool isAddrSpaceMapManglingEnabled(const TargetInfo ,
   const LangOptions ) {
   switch (LangOpts.getAddressSpaceMapMangling()) {
@@ -1293,7 +1260,6 @@
   this->AuxTarget = AuxTarget;
 
   ABI.reset(createCXXABI(Target));
-  AddrSpaceMap = getAddressSpaceMap(Target, LangOpts);
   AddrSpaceMapMangling = isAddrSpaceMapManglingEnabled(Target, LangOpts);
 
   // C99 6.2.5p19.
@@ -12244,10 +12210,7 @@
 }
 
 unsigned ASTContext::getTargetAddressSpace(LangAS AS) const {
-  if (isTargetAddressSpace(AS))
-return toTargetAddressSpace(AS);
-  else
-return (*AddrSpaceMap)[(unsigned)AS];
+  return getTargetInfo().getTargetAddressSpace(AS);
 }
 
 bool ASTContext::hasSameExpr(const Expr *X, const Expr *Y) const {
Index: clang/include/clang/AST/ASTContext.h
===
--- clang/include/clang/AST/ASTContext.h
+++ clang/include/clang/AST/ASTContext.h
@@ -613,9 +613,6 @@
   std::unique_ptr ABI;
   CXXABI *createCXXABI(const TargetInfo );
 
-  /// The logical -> physical address space map.
-  const LangASMap *AddrSpaceMap = nullptr;
-
   /// Address space map mangling must be used with language specific
   /// address spaces (e.g. OpenCL/CUDA)
   bool AddrSpaceMapMangling;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D138722: Overload all llvm.annotation intrinsics for globals argument

[Alexander Richardson via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
arichardson marked 2 inline comments as done.
Closed by commit rG9114ac67a986: Overload all llvm.annotation intrinsics for 
globals argument (authored by arichardson).

Changed prior to commit:
  https://reviews.llvm.org/D138722?vs=480900=480969#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138722

Files:
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/CodeGen/CodeGenTypeCache.h
  clang/test/CodeGen/annotations-field.c
  clang/test/CodeGen/annotations-global.c
  clang/test/CodeGen/annotations-loc.c
  clang/test/CodeGen/annotations-var.c
  clang/test/CodeGenCXX/attr-annotate.cpp
  clang/test/CodeGenCXX/attr-annotate2.cpp
  clang/test/CodeGenSYCL/field-annotate-addr-space.cpp
  llvm/include/llvm/IR/Intrinsics.td
  llvm/lib/IR/AutoUpgrade.cpp
  llvm/test/Analysis/CostModel/X86/free-intrinsics.ll
  llvm/test/Analysis/CostModel/free-intrinsics-datalayout.ll
  llvm/test/Analysis/CostModel/free-intrinsics-no_info.ll
  llvm/test/Analysis/ScalarEvolution/annotation-intrinsics.ll
  llvm/test/Assembler/opaque-ptr-intrinsic-remangling.ll
  llvm/test/Bitcode/upgrade-annotation.ll
  llvm/test/Bitcode/upgrade-ptr-annotation.ll
  llvm/test/Bitcode/upgrade-var-annotation.ll
  llvm/test/Transforms/InstCombine/annotation-intrinsic.ll
  llvm/test/Transforms/InstCombine/assume_inevitable.ll

Index: llvm/test/Transforms/InstCombine/assume_inevitable.ll
===
--- llvm/test/Transforms/InstCombine/assume_inevitable.ll
+++ llvm/test/Transforms/InstCombine/assume_inevitable.ll
@@ -10,11 +10,11 @@
 ; CHECK-NEXT:[[M:%.*]] = alloca i64, align 8
 ; CHECK-NEXT:[[TMP0:%.*]] = load i32, ptr [[A:%.*]], align 32
 ; CHECK-NEXT:[[LOADRES:%.*]] = load i32, ptr [[B:%.*]], align 4
-; CHECK-NEXT:[[LOADRES2:%.*]] = call i32 @llvm.annotation.i32(i32 [[LOADRES]], ptr nonnull @.str, ptr nonnull @.str1, i32 2)
+; CHECK-NEXT:[[LOADRES2:%.*]] = call i32 @llvm.annotation.i32.p0(i32 [[LOADRES]], ptr nonnull @.str, ptr nonnull @.str1, i32 2)
 ; CHECK-NEXT:store i32 [[LOADRES2]], ptr [[A]], align 32
 ; CHECK-NEXT:[[DUMMY_EQ:%.*]] = icmp ugt i32 [[LOADRES]], 42
 ; CHECK-NEXT:tail call void @llvm.assume(i1 [[DUMMY_EQ]])
-; CHECK-NEXT:[[M_A:%.*]] = call ptr @llvm.ptr.annotation.p0(ptr nonnull [[M]], ptr nonnull @.str, ptr nonnull @.str1, i32 2, ptr null)
+; CHECK-NEXT:[[M_A:%.*]] = call ptr @llvm.ptr.annotation.p0.p0(ptr nonnull [[M]], ptr nonnull @.str, ptr nonnull @.str1, i32 2, ptr null)
 ; CHECK-NEXT:[[OBJSZ:%.*]] = call i64 @llvm.objectsize.i64.p0(ptr [[C:%.*]], i1 false, i1 false, i1 false)
 ; CHECK-NEXT:store i64 [[OBJSZ]], ptr [[M_A]], align 4
 ; CHECK-NEXT:[[PTRINT:%.*]] = ptrtoint ptr [[A]] to i64
Index: llvm/test/Transforms/InstCombine/annotation-intrinsic.ll
===
--- llvm/test/Transforms/InstCombine/annotation-intrinsic.ll
+++ llvm/test/Transforms/InstCombine/annotation-intrinsic.ll
@@ -12,7 +12,7 @@
 ; CHECK-LABEL: @annotated(
 ; CHECK-NEXT:  entry:
 ; CHECK-NEXT:[[TMP0:%.*]] = load i32, ptr [[C:%.*]], align 4
-; CHECK-NEXT:[[TMP1:%.*]] = call i32 @llvm.annotation.i32(i32 [[TMP0]], ptr undef, ptr undef, i32 undef)
+; CHECK-NEXT:[[TMP1:%.*]] = call i32 @llvm.annotation.i32.p0(i32 [[TMP0]], ptr undef, ptr undef, i32 undef)
 ; CHECK-NEXT:[[ADD:%.*]] = add nsw i32 [[TMP1]], [[TMP0]]
 ; CHECK-NEXT:ret i32 [[ADD]]
 ;
Index: llvm/test/Bitcode/upgrade-var-annotation.ll
===
--- llvm/test/Bitcode/upgrade-var-annotation.ll
+++ llvm/test/Bitcode/upgrade-var-annotation.ll
@@ -7,10 +7,10 @@
 define void @f(i8* %arg0, i8* %arg1, i8* %arg2, i32 %arg3) {
 ;CHECK: @f(i8* [[ARG0:%.*]], i8* [[ARG1:%.*]], i8* [[ARG2:%.*]], i32 [[ARG3:%.*]])
   call void @llvm.var.annotation(i8* %arg0, i8* %arg1, i8* %arg2, i32 %arg3)
-;CHECK:  call void @llvm.var.annotation(i8* [[ARG0]], i8* [[ARG1]], i8* [[ARG2]], i32 [[ARG3]], i8* null)
+;CHECK:  call void @llvm.var.annotation.p0i8.p0i8(i8* [[ARG0]], i8* [[ARG1]], i8* [[ARG2]], i32 [[ARG3]], i8* null)
   ret void
 }
 
 ; Function Attrs: nofree nosync nounwind willreturn
 declare void @llvm.var.annotation(i8*, i8*, i8*, i32)
-; CHECK: declare void @llvm.var.annotation(i8*, i8*, i8*, i32, i8*)
+; CHECK: declare void @llvm.var.annotation.p0i8.p0i8(i8*, i8*, i8*, i32, i8*)
Index: llvm/test/Bitcode/upgrade-ptr-annotation.ll
===
--- llvm/test/Bitcode/upgrade-ptr-annotation.ll
+++ llvm/test/Bitcode/upgrade-ptr-annotation.ll
@@ -10,17 +10,17 @@
 define void @f1(i8* %arg0, i8* %arg1, i8* %arg2, i32 %arg3) {
 ;CHECK: @f1(i8* 

[PATCH] D138722: Overload all llvm.annotation intrinsics for globals argument

[Alexander Richardson via Phabricator via cfe-commits]

arichardson updated this revision to Diff 480900.
arichardson added a comment.

use opaque pointers in the new test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138722

Files:
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/CodeGen/CodeGenTypeCache.h
  clang/test/CodeGen/annotations-field.c
  clang/test/CodeGen/annotations-global.c
  clang/test/CodeGen/annotations-loc.c
  clang/test/CodeGen/annotations-var.c
  clang/test/CodeGenCXX/attr-annotate.cpp
  clang/test/CodeGenCXX/attr-annotate2.cpp
  clang/test/CodeGenSYCL/field-annotate-addr-space.cpp
  llvm/include/llvm/IR/Intrinsics.td
  llvm/lib/IR/AutoUpgrade.cpp
  llvm/test/Analysis/CostModel/X86/free-intrinsics.ll
  llvm/test/Analysis/CostModel/free-intrinsics-datalayout.ll
  llvm/test/Analysis/CostModel/free-intrinsics-no_info.ll
  llvm/test/Analysis/ScalarEvolution/annotation-intrinsics.ll
  llvm/test/Assembler/opaque-ptr-intrinsic-remangling.ll
  llvm/test/Bitcode/upgrade-annotation.ll
  llvm/test/Bitcode/upgrade-annotation.ll.bc
  llvm/test/Bitcode/upgrade-ptr-annotation.ll
  llvm/test/Bitcode/upgrade-var-annotation.ll
  llvm/test/Transforms/InstCombine/annotation-intrinsic.ll
  llvm/test/Transforms/InstCombine/assume_inevitable.ll

Index: llvm/test/Transforms/InstCombine/assume_inevitable.ll
===
--- llvm/test/Transforms/InstCombine/assume_inevitable.ll
+++ llvm/test/Transforms/InstCombine/assume_inevitable.ll
@@ -10,11 +10,11 @@
 ; CHECK-NEXT:[[M:%.*]] = alloca i64, align 8
 ; CHECK-NEXT:[[TMP0:%.*]] = load i32, ptr [[A:%.*]], align 32
 ; CHECK-NEXT:[[LOADRES:%.*]] = load i32, ptr [[B:%.*]], align 4
-; CHECK-NEXT:[[LOADRES2:%.*]] = call i32 @llvm.annotation.i32(i32 [[LOADRES]], ptr nonnull @.str, ptr nonnull @.str1, i32 2)
+; CHECK-NEXT:[[LOADRES2:%.*]] = call i32 @llvm.annotation.i32.p0(i32 [[LOADRES]], ptr nonnull @.str, ptr nonnull @.str1, i32 2)
 ; CHECK-NEXT:store i32 [[LOADRES2]], ptr [[A]], align 32
 ; CHECK-NEXT:[[DUMMY_EQ:%.*]] = icmp ugt i32 [[LOADRES]], 42
 ; CHECK-NEXT:tail call void @llvm.assume(i1 [[DUMMY_EQ]])
-; CHECK-NEXT:[[M_A:%.*]] = call ptr @llvm.ptr.annotation.p0(ptr nonnull [[M]], ptr nonnull @.str, ptr nonnull @.str1, i32 2, ptr null)
+; CHECK-NEXT:[[M_A:%.*]] = call ptr @llvm.ptr.annotation.p0.p0(ptr nonnull [[M]], ptr nonnull @.str, ptr nonnull @.str1, i32 2, ptr null)
 ; CHECK-NEXT:[[OBJSZ:%.*]] = call i64 @llvm.objectsize.i64.p0(ptr [[C:%.*]], i1 false, i1 false, i1 false)
 ; CHECK-NEXT:store i64 [[OBJSZ]], ptr [[M_A]], align 4
 ; CHECK-NEXT:[[PTRINT:%.*]] = ptrtoint ptr [[A]] to i64
Index: llvm/test/Transforms/InstCombine/annotation-intrinsic.ll
===
--- llvm/test/Transforms/InstCombine/annotation-intrinsic.ll
+++ llvm/test/Transforms/InstCombine/annotation-intrinsic.ll
@@ -12,7 +12,7 @@
 ; CHECK-LABEL: @annotated(
 ; CHECK-NEXT:  entry:
 ; CHECK-NEXT:[[TMP0:%.*]] = load i32, ptr [[C:%.*]], align 4
-; CHECK-NEXT:[[TMP1:%.*]] = call i32 @llvm.annotation.i32(i32 [[TMP0]], ptr undef, ptr undef, i32 undef)
+; CHECK-NEXT:[[TMP1:%.*]] = call i32 @llvm.annotation.i32.p0(i32 [[TMP0]], ptr undef, ptr undef, i32 undef)
 ; CHECK-NEXT:[[ADD:%.*]] = add nsw i32 [[TMP1]], [[TMP0]]
 ; CHECK-NEXT:ret i32 [[ADD]]
 ;
Index: llvm/test/Bitcode/upgrade-var-annotation.ll
===
--- llvm/test/Bitcode/upgrade-var-annotation.ll
+++ llvm/test/Bitcode/upgrade-var-annotation.ll
@@ -1,15 +1,16 @@
 ; Test upgrade of var.annotation intrinsics.
 ;
+; RUN: llvm-as < %s | llvm-dis | FileCheck %s
 ; RUN: llvm-dis < %s.bc | FileCheck %s
 
 
 define void @f(i8* %arg0, i8* %arg1, i8* %arg2, i32 %arg3) {
 ;CHECK: @f(i8* [[ARG0:%.*]], i8* [[ARG1:%.*]], i8* [[ARG2:%.*]], i32 [[ARG3:%.*]])
   call void @llvm.var.annotation(i8* %arg0, i8* %arg1, i8* %arg2, i32 %arg3)
-;CHECK:  call void @llvm.var.annotation(i8* [[ARG0]], i8* [[ARG1]], i8* [[ARG2]], i32 [[ARG3]], i8* null)
+;CHECK:  call void @llvm.var.annotation.p0i8.p0i8(i8* [[ARG0]], i8* [[ARG1]], i8* [[ARG2]], i32 [[ARG3]], i8* null)
   ret void
 }
 
 ; Function Attrs: nofree nosync nounwind willreturn
 declare void @llvm.var.annotation(i8*, i8*, i8*, i32)
-; CHECK: declare void @llvm.var.annotation(i8*, i8*, i8*, i32, i8*)
+; CHECK: declare void @llvm.var.annotation.p0i8.p0i8(i8*, i8*, i8*, i32, i8*)
Index: llvm/test/Bitcode/upgrade-ptr-annotation.ll
===
--- llvm/test/Bitcode/upgrade-ptr-annotation.ll
+++ llvm/test/Bitcode/upgrade-ptr-annotation.ll
@@ -1,5 +1,6 @@
 ; Test upgrade of ptr.annotation intrinsics.
 ;
+; RUN: llvm-as < %s | llvm-dis | FileCheck %s
 ; RUN: llvm-dis < %s.bc | FileCheck %s
 
 ; Unused return values
@@ 

[PATCH] D139305: [clang][driver] Support option '-mcpu' on target AVR

[Alexander Richardson via Phabricator via cfe-commits]

arichardson added inline comments.



Comment at: clang/lib/Driver/ToolChains/CommonArgs.cpp:387
   return A->getValue();
+if (const Arg *A = Args.getLastArg(options::OPT_mcpu_EQ))
+  return A->getValue();

It looks to me that -mcpu will always take precedence over -mmcu with this code 
even if mmcu comes later in the command line. Wouldn't it make more sense to 
treat them as aliases?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139305

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

[PATCH] D138296: [clang] Avoid duplicating ProgramAddressSpace in TargetInfo. NFCI

[Alexander Richardson via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGf3a17d059509: [clang] Avoid duplicating ProgramAddressSpace 
in TargetInfo. NFCI (authored by arichardson).

Changed prior to commit:
  https://reviews.llvm.org/D138296?vs=476473=479387#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138296

Files:
  clang/include/clang/AST/ASTContext.h
  clang/include/clang/Basic/TargetInfo.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/Basic/TargetInfo.cpp
  clang/lib/Basic/Targets/AVR.h
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/lib/CodeGen/CGDeclCXX.cpp
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/CodeGen/CodeGenTypes.cpp
  clang/lib/CodeGen/CodeGenTypes.h

Index: clang/lib/CodeGen/CodeGenTypes.h
===
--- clang/lib/CodeGen/CodeGenTypes.h
+++ clang/lib/CodeGen/CodeGenTypes.h
@@ -305,7 +305,7 @@
   bool isRecordBeingLaidOut(const Type *Ty) const {
 return RecordsBeingLaidOut.count(Ty);
   }
-
+  unsigned getTargetAddressSpace(QualType T) const;
 };
 
 }  // end namespace CodeGen
Index: clang/lib/CodeGen/CodeGenTypes.cpp
===
--- clang/lib/CodeGen/CodeGenTypes.cpp
+++ clang/lib/CodeGen/CodeGenTypes.cpp
@@ -655,7 +655,7 @@
 const ReferenceType *RTy = cast(Ty);
 QualType ETy = RTy->getPointeeType();
 llvm::Type *PointeeType = ConvertTypeForMem(ETy);
-unsigned AS = Context.getTargetAddressSpace(ETy);
+unsigned AS = getTargetAddressSpace(ETy);
 ResultType = llvm::PointerType::get(PointeeType, AS);
 break;
   }
@@ -665,7 +665,7 @@
 llvm::Type *PointeeType = ConvertTypeForMem(ETy);
 if (PointeeType->isVoidTy())
   PointeeType = llvm::Type::getInt8Ty(getLLVMContext());
-unsigned AS = Context.getTargetAddressSpace(ETy);
+unsigned AS = getTargetAddressSpace(ETy);
 ResultType = llvm::PointerType::get(PointeeType, AS);
 break;
   }
@@ -958,3 +958,13 @@
 bool CodeGenTypes::isZeroInitializable(const RecordDecl *RD) {
   return getCGRecordLayout(RD).isZeroInitializable();
 }
+
+unsigned CodeGenTypes::getTargetAddressSpace(QualType T) const {
+  // Return the address space for the type. If the type is a
+  // function type without an address space qualifier, the
+  // program address space is used. Otherwise, the target picks
+  // the best address space based on the type information
+  return T->isFunctionType() && !T.hasAddressSpace()
+ ? getDataLayout().getProgramAddressSpace()
+ : getContext().getTargetAddressSpace(T.getAddressSpace());
+}
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -3196,7 +3196,7 @@
   // See if there is already something with the target's name in the module.
   llvm::GlobalValue *Entry = GetGlobalValue(AA->getAliasee());
   if (Entry) {
-unsigned AS = getContext().getTargetAddressSpace(VD->getType());
+unsigned AS = getTypes().getTargetAddressSpace(VD->getType());
 auto Ptr = llvm::ConstantExpr::getBitCast(Entry, DeclTy->getPointerTo(AS));
 return ConstantAddress(Ptr, DeclTy, Alignment);
   }
@@ -3761,7 +3761,7 @@
   if (getTarget().supportsIFunc()) {
 ResolverType = llvm::FunctionType::get(
 llvm::PointerType::get(DeclTy,
-   Context.getTargetAddressSpace(FD->getType())),
+   getTypes().getTargetAddressSpace(FD->getType())),
 false);
   }
   else {
@@ -3899,8 +3899,8 @@
   // cpu_dispatch will be emitted in this translation unit.
   if (getTarget().supportsIFunc() && !FD->isCPUSpecificMultiVersion()) {
 llvm::Type *ResolverType = llvm::FunctionType::get(
-llvm::PointerType::get(
-DeclTy, getContext().getTargetAddressSpace(FD->getType())),
+llvm::PointerType::get(DeclTy,
+   getTypes().getTargetAddressSpace(FD->getType())),
 false);
 llvm::Constant *Resolver = GetOrCreateLLVMFunction(
 MangledName + ".resolver", ResolverType, GlobalDecl{},
Index: clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp
===
--- clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp
+++ clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp
@@ -3133,7 +3133,7 @@
   const Type *NonQualTy = QC.strip(NativeParamType);
   QualType NativePointeeTy = cast(NonQualTy)->getPointeeType();
   unsigned NativePointeeAddrSpace =
-  CGF.getContext().getTargetAddressSpace(NativePointeeTy);
+  CGF.getTypes().getTargetAddressSpace(NativePointeeTy);
   QualType TargetTy = TargetParam->getType();
   llvm::Value *TargetAddr = CGF.EmitLoadOfScalar(
   LocalAddr, 

[PATCH] D138296: [clang] Avoid duplicating ProgramAddressSpace in TargetInfo. NFCI

[Alexander Richardson via Phabricator via cfe-commits]

arichardson added a comment.

In D138296#3937599 , @rjmccall wrote:

> In D138296#3937486 , @arichardson 
> wrote:
>
>> In D138296#3937224 , @eandrews 
>> wrote:
>>
>>> Functionally this looks ok to me. However I am not sure if CodeGenTypes is 
>>> the 'right' place for this function to live, considering that other 
>>> functions with similar functionality are in ASTContext - including 
>>> overloads of getTargetAddressSpace( ). @erichkeane @aaron.ballman could you 
>>> please take a look?
>>
>> My view is that the parts that interact with LLVM IR should really live in 
>> CodeGen/ and not Basic/ or AST/. I will see how difficult it would be to 
>> move the remaining target (LLVM IR) address space handling code to CodeGen/
>
> Yeah, I don't think there's a good reason for some of the address-space stuff 
> that currently lives in Basic to be there instead of in CodeGen.  Basic/AST 
> need to understand what address spaces exist, their sizes, and what 
> relationships they have with each other, and that's it.

Thanks for looking at this - does this mean you are happy for me to commit this 
change?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138296

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

[PATCH] D138295: [clang][TargetInfo] Use LangAS for getPointer{Width,Align}()

[Alexander Richardson via Phabricator via cfe-commits]

arichardson added inline comments.



Comment at: clang/lib/AST/ASTContext.cpp:2228-2230
+  Width = Target->getPointerWidth(
+  LangAS::Default); // C++ 3.9.1p11: sizeof(nullptr_t)
+  Align = Target->getPointerAlign(LangAS::Default); //   == sizeof(void*)

jrtc27 wrote:
> clang-format(?) screwed up this comment, maybe better to just put it all on 
> one line before these rather than as inline?
Thanks, fixed now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138295

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

[PATCH] D138295: [clang][TargetInfo] Use LangAS for getPointer{Width,Align}()

[Alexander Richardson via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
arichardson marked an inline comment as done.
Closed by commit rGa602f76a2406: [clang][TargetInfo] Use LangAS for 
getPointer{Width,Align}() (authored by arichardson).

Changed prior to commit:
  https://reviews.llvm.org/D138295?vs=478867=479048#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138295

Files:
  clang/include/clang/Basic/TargetInfo.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/AST/ItaniumCXXABI.cpp
  clang/lib/AST/Mangle.cpp
  clang/lib/AST/MicrosoftCXXABI.cpp
  clang/lib/AST/MicrosoftMangle.cpp
  clang/lib/AST/RecordLayoutBuilder.cpp
  clang/lib/AST/VTableBuilder.cpp
  clang/lib/Basic/Targets/AMDGPU.cpp
  clang/lib/Basic/Targets/AMDGPU.h
  clang/lib/Basic/Targets/Mips.cpp
  clang/lib/Basic/Targets/NVPTX.cpp
  clang/lib/Basic/Targets/X86.h
  clang/lib/CodeGen/CGBlocks.cpp
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/lib/CodeGen/CGExprAgg.cpp
  clang/lib/CodeGen/CGObjCGNU.cpp
  clang/lib/CodeGen/CGObjCMac.cpp
  clang/lib/CodeGen/CGVTables.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/CodeGen/ItaniumCXXABI.cpp
  clang/lib/CodeGen/MicrosoftCXXABI.cpp
  clang/lib/CodeGen/PatternInit.cpp
  clang/lib/CodeGen/SwiftCallingConv.cpp
  clang/lib/CodeGen/TargetInfo.cpp
  clang/lib/Frontend/InitPreprocessor.cpp
  clang/lib/Sema/SemaChecking.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/lib/Sema/SemaType.cpp
  clang/lib/StaticAnalyzer/Checkers/ObjCContainersASTChecker.cpp
  clang/unittests/CodeGen/TestCompiler.h

Index: clang/unittests/CodeGen/TestCompiler.h
===
--- clang/unittests/CodeGen/TestCompiler.h
+++ clang/unittests/CodeGen/TestCompiler.h
@@ -48,7 +48,7 @@
 std::make_shared(compiler.getTargetOpts(;
 
 const clang::TargetInfo  = compiler.getTarget();
-PtrSize = TInfo.getPointerWidth(0) / 8;
+PtrSize = TInfo.getPointerWidth(clang::LangAS::Default) / 8;
 
 compiler.createFileManager();
 compiler.createSourceManager(compiler.getFileManager());
Index: clang/lib/StaticAnalyzer/Checkers/ObjCContainersASTChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/ObjCContainersASTChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/ObjCContainersASTChecker.cpp
@@ -72,7 +72,7 @@
 public:
   WalkAST(BugReporter , const CheckerBase *checker, AnalysisDeclContext *ac)
   : BR(br), Checker(checker), AC(ac), ASTC(AC->getASTContext()),
-PtrWidth(ASTC.getTargetInfo().getPointerWidth(0)) {}
+PtrWidth(ASTC.getTargetInfo().getPointerWidth(LangAS::Default)) {}
 
   // Statement visitor methods.
   void VisitChildren(Stmt *S);
Index: clang/lib/Sema/SemaType.cpp
===
--- clang/lib/Sema/SemaType.cpp
+++ clang/lib/Sema/SemaType.cpp
@@ -7288,7 +7288,8 @@
 
   // Add address space to type based on its attributes.
   LangAS ASIdx = LangAS::Default;
-  uint64_t PtrWidth = S.Context.getTargetInfo().getPointerWidth(0);
+  uint64_t PtrWidth =
+  S.Context.getTargetInfo().getPointerWidth(LangAS::Default);
   if (PtrWidth == 32) {
 if (Attrs[attr::Ptr64])
   ASIdx = LangAS::ptr64;
Index: clang/lib/Sema/SemaExprCXX.cpp
===
--- clang/lib/Sema/SemaExprCXX.cpp
+++ clang/lib/Sema/SemaExprCXX.cpp
@@ -2661,7 +2661,9 @@
   // tree? Or should the consumer just recalculate the value?
   // FIXME: Using a dummy value will interact poorly with attribute enable_if.
   IntegerLiteral Size(
-  Context, llvm::APInt::getZero(Context.getTargetInfo().getPointerWidth(0)),
+  Context,
+  llvm::APInt::getZero(
+  Context.getTargetInfo().getPointerWidth(LangAS::Default)),
   Context.getSizeType(), SourceLocation());
   AllocArgs.push_back();
 
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -16707,7 +16707,7 @@
   // The type of __null will be int or long, depending on the size of
   // pointers on the target.
   QualType Ty;
-  unsigned pw = Context.getTargetInfo().getPointerWidth(0);
+  unsigned pw = Context.getTargetInfo().getPointerWidth(LangAS::Default);
   if (pw == Context.getTargetInfo().getIntWidth())
 Ty = Context.IntTy;
   else if (pw == Context.getTargetInfo().getLongWidth())
Index: clang/lib/Sema/SemaDeclAttr.cpp
===
--- clang/lib/Sema/SemaDeclAttr.cpp
+++ clang/lib/Sema/SemaDeclAttr.cpp
@@ -4506,7 +4506,7 @@
 break;
   case 7:
 if (Str == "pointer")
-  DestWidth = S.Context.getTargetInfo().getPointerWidth(0);
+  DestWidth = 

[PATCH] D139006: [UpdateTestChecks] Match define for labels

[Alexander Richardson via Phabricator via cfe-commits]

arichardson added a comment.

Agreed that the churn is annoying, but at least unlike the function-signature 
flag (which I'd quite like to have on by default tbh) it only affects a single 
line rather than also including variable captures.




Comment at: 
llvm/test/tools/UpdateTestChecks/update_test_checks/Inputs/define_after_call.ll.expected:14
+define i32 @b(i32 %arg) {
+; CHECK-LABEL: define {{[^@]+}}@b(
+; CHECK-NEXT:ret i32 [[ARG:%.*]]

Does this actually fail without the define match? I wouldn't expect it to?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139006

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

[PATCH] D138295: [clang][TargetInfo] Use LangAS for getPointer{Width,Align}()

[Alexander Richardson via Phabricator via cfe-commits]

arichardson added inline comments.



Comment at: clang/lib/AST/RecordLayoutBuilder.cpp:1917
+EffectiveFieldSize = FieldSize = TI.Width;
+FieldAlign = TI.Align;
   } else {

rjmccall wrote:
> This is a nice cleanup, but I actually can't figure out why we can't just 
> fall into the code below.
It looks like this dates back to `Add a new ASTRecordLayoutBuilder class. Not 
used yet.` from 2009. Maybe `ReferenceType`s were not handled correctly by 
`Context.getTypeInfoInChars(RT);` back then?



Comment at: clang/lib/AST/RecordLayoutBuilder.cpp:1917
+EffectiveFieldSize = FieldSize = TI.Width;
+FieldAlign = TI.Align;
   } else {

arichardson wrote:
> rjmccall wrote:
> > This is a nice cleanup, but I actually can't figure out why we can't just 
> > fall into the code below.
> It looks like this dates back to `Add a new ASTRecordLayoutBuilder class. Not 
> used yet.` from 2009. Maybe `ReferenceType`s were not handled correctly by 
> `Context.getTypeInfoInChars(RT);` back then?
I've removed the special case now and it looks like all tests are passing, so I 
believe this is indeed no longer needed.



Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:1706
+assert(ThisPtrTy->getPointeeType().getAddressSpace() == LangAS::Default &&
+   "this pointer not in default address space?");
 auto Align = getTypeAlignIfRequired(ThisPtrTy, CGM.getContext());

rjmccall wrote:
> I'm pretty sure we have language modes that support methods in different 
> address spaces, and your code doesn't seem to require this assertion.
I added this assertion to see if we have any tests for this case and forgot to 
remove it. I think it should hopefully be correct, so I've removed the 
assertion.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138295

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

[PATCH] D138316: [ASTContext] Avoid duplicating address space map. NFCI

[Alexander Richardson via Phabricator via cfe-commits]

arichardson updated this revision to Diff 478870.
arichardson added a comment.

rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138316

Files:
  clang/include/clang/AST/ASTContext.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/Basic/TargetInfo.cpp

Index: clang/lib/Basic/TargetInfo.cpp
===
--- clang/lib/Basic/TargetInfo.cpp
+++ clang/lib/Basic/TargetInfo.cpp
@@ -24,6 +24,30 @@
 using namespace clang;
 
 static const LangASMap DefaultAddrSpaceMap = {0};
+// The fake address space map must have a distinct entry for each
+// language-specific address space.
+static const LangASMap FakeAddrSpaceMap = {
+0,  // Default
+1,  // opencl_global
+3,  // opencl_local
+2,  // opencl_constant
+0,  // opencl_private
+4,  // opencl_generic
+5,  // opencl_global_device
+6,  // opencl_global_host
+7,  // cuda_device
+8,  // cuda_constant
+9,  // cuda_shared
+1,  // sycl_global
+5,  // sycl_global_device
+6,  // sycl_global_host
+3,  // sycl_local
+0,  // sycl_private
+10, // ptr32_sptr
+11, // ptr32_uptr
+12, // ptr64
+13, // hlsl_groupshared
+};
 
 // TargetInfo Constructor.
 TargetInfo::TargetInfo(const llvm::Triple ) : Triple(T) {
@@ -487,6 +511,9 @@
 
   if (Opts.MaxBitIntWidth)
 MaxBitIntWidth = Opts.MaxBitIntWidth;
+
+  if (Opts.FakeAddressSpaceMap)
+AddrSpaceMap = 
 }
 
 bool TargetInfo::initFeatureMap(
Index: clang/lib/AST/ASTContext.cpp
===
--- clang/lib/AST/ASTContext.cpp
+++ clang/lib/AST/ASTContext.cpp
@@ -929,39 +929,6 @@
   return *ParentMapCtx.get();
 }
 
-static const LangASMap *getAddressSpaceMap(const TargetInfo ,
-   const LangOptions ) {
-  if (LOpts.FakeAddressSpaceMap) {
-// The fake address space map must have a distinct entry for each
-// language-specific address space.
-static const unsigned FakeAddrSpaceMap[] = {
-0,  // Default
-1,  // opencl_global
-3,  // opencl_local
-2,  // opencl_constant
-0,  // opencl_private
-4,  // opencl_generic
-5,  // opencl_global_device
-6,  // opencl_global_host
-7,  // cuda_device
-8,  // cuda_constant
-9,  // cuda_shared
-1,  // sycl_global
-5,  // sycl_global_device
-6,  // sycl_global_host
-3,  // sycl_local
-0,  // sycl_private
-10, // ptr32_sptr
-11, // ptr32_uptr
-12, // ptr64
-13, // hlsl_groupshared
-};
-return 
-  } else {
-return ();
-  }
-}
-
 static bool isAddrSpaceMapManglingEnabled(const TargetInfo ,
   const LangOptions ) {
   switch (LangOpts.getAddressSpaceMapMangling()) {
@@ -1292,7 +1259,6 @@
   this->AuxTarget = AuxTarget;
 
   ABI.reset(createCXXABI(Target));
-  AddrSpaceMap = getAddressSpaceMap(Target, LangOpts);
   AddrSpaceMapMangling = isAddrSpaceMapManglingEnabled(Target, LangOpts);
 
   // C99 6.2.5p19.
@@ -12228,10 +12194,7 @@
 }
 
 unsigned ASTContext::getTargetAddressSpace(LangAS AS) const {
-  if (isTargetAddressSpace(AS))
-return toTargetAddressSpace(AS);
-  else
-return (*AddrSpaceMap)[(unsigned)AS];
+  return getTargetInfo().getTargetAddressSpace(AS);
 }
 
 bool ASTContext::hasSameExpr(const Expr *X, const Expr *Y) const {
Index: clang/include/clang/AST/ASTContext.h
===
--- clang/include/clang/AST/ASTContext.h
+++ clang/include/clang/AST/ASTContext.h
@@ -640,9 +640,6 @@
   std::unique_ptr ABI;
   CXXABI *createCXXABI(const TargetInfo );
 
-  /// The logical -> physical address space map.
-  const LangASMap *AddrSpaceMap = nullptr;
-
   /// Address space map mangling must be used with language specific
   /// address spaces (e.g. OpenCL/CUDA)
   bool AddrSpaceMapMangling;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D138295: [clang][TargetInfo] Use LangAS for getPointer{Width,Align}()

[Alexander Richardson via Phabricator via cfe-commits]

arichardson updated this revision to Diff 478867.
arichardson marked 3 inline comments as done.
arichardson added a comment.

address review feedback


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138295

Files:
  clang/include/clang/Basic/TargetInfo.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/AST/ItaniumCXXABI.cpp
  clang/lib/AST/Mangle.cpp
  clang/lib/AST/MicrosoftCXXABI.cpp
  clang/lib/AST/MicrosoftMangle.cpp
  clang/lib/AST/RecordLayoutBuilder.cpp
  clang/lib/AST/VTableBuilder.cpp
  clang/lib/Basic/Targets/AMDGPU.cpp
  clang/lib/Basic/Targets/AMDGPU.h
  clang/lib/Basic/Targets/Mips.cpp
  clang/lib/Basic/Targets/NVPTX.cpp
  clang/lib/Basic/Targets/X86.h
  clang/lib/CodeGen/CGBlocks.cpp
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/lib/CodeGen/CGExprAgg.cpp
  clang/lib/CodeGen/CGObjCGNU.cpp
  clang/lib/CodeGen/CGObjCMac.cpp
  clang/lib/CodeGen/CGVTables.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/CodeGen/ItaniumCXXABI.cpp
  clang/lib/CodeGen/MicrosoftCXXABI.cpp
  clang/lib/CodeGen/PatternInit.cpp
  clang/lib/CodeGen/SwiftCallingConv.cpp
  clang/lib/CodeGen/TargetInfo.cpp
  clang/lib/Frontend/InitPreprocessor.cpp
  clang/lib/Sema/SemaChecking.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/lib/Sema/SemaType.cpp
  clang/lib/StaticAnalyzer/Checkers/ObjCContainersASTChecker.cpp
  clang/unittests/CodeGen/TestCompiler.h

Index: clang/unittests/CodeGen/TestCompiler.h
===
--- clang/unittests/CodeGen/TestCompiler.h
+++ clang/unittests/CodeGen/TestCompiler.h
@@ -48,7 +48,7 @@
 std::make_shared(compiler.getTargetOpts(;
 
 const clang::TargetInfo  = compiler.getTarget();
-PtrSize = TInfo.getPointerWidth(0) / 8;
+PtrSize = TInfo.getPointerWidth(clang::LangAS::Default) / 8;
 
 compiler.createFileManager();
 compiler.createSourceManager(compiler.getFileManager());
Index: clang/lib/StaticAnalyzer/Checkers/ObjCContainersASTChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/ObjCContainersASTChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/ObjCContainersASTChecker.cpp
@@ -72,7 +72,7 @@
 public:
   WalkAST(BugReporter , const CheckerBase *checker, AnalysisDeclContext *ac)
   : BR(br), Checker(checker), AC(ac), ASTC(AC->getASTContext()),
-PtrWidth(ASTC.getTargetInfo().getPointerWidth(0)) {}
+PtrWidth(ASTC.getTargetInfo().getPointerWidth(LangAS::Default)) {}
 
   // Statement visitor methods.
   void VisitChildren(Stmt *S);
Index: clang/lib/Sema/SemaType.cpp
===
--- clang/lib/Sema/SemaType.cpp
+++ clang/lib/Sema/SemaType.cpp
@@ -7288,7 +7288,8 @@
 
   // Add address space to type based on its attributes.
   LangAS ASIdx = LangAS::Default;
-  uint64_t PtrWidth = S.Context.getTargetInfo().getPointerWidth(0);
+  uint64_t PtrWidth =
+  S.Context.getTargetInfo().getPointerWidth(LangAS::Default);
   if (PtrWidth == 32) {
 if (Attrs[attr::Ptr64])
   ASIdx = LangAS::ptr64;
Index: clang/lib/Sema/SemaExprCXX.cpp
===
--- clang/lib/Sema/SemaExprCXX.cpp
+++ clang/lib/Sema/SemaExprCXX.cpp
@@ -2661,7 +2661,9 @@
   // tree? Or should the consumer just recalculate the value?
   // FIXME: Using a dummy value will interact poorly with attribute enable_if.
   IntegerLiteral Size(
-  Context, llvm::APInt::getZero(Context.getTargetInfo().getPointerWidth(0)),
+  Context,
+  llvm::APInt::getZero(
+  Context.getTargetInfo().getPointerWidth(LangAS::Default)),
   Context.getSizeType(), SourceLocation());
   AllocArgs.push_back();
 
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -16707,7 +16707,7 @@
   // The type of __null will be int or long, depending on the size of
   // pointers on the target.
   QualType Ty;
-  unsigned pw = Context.getTargetInfo().getPointerWidth(0);
+  unsigned pw = Context.getTargetInfo().getPointerWidth(LangAS::Default);
   if (pw == Context.getTargetInfo().getIntWidth())
 Ty = Context.IntTy;
   else if (pw == Context.getTargetInfo().getLongWidth())
Index: clang/lib/Sema/SemaDeclAttr.cpp
===
--- clang/lib/Sema/SemaDeclAttr.cpp
+++ clang/lib/Sema/SemaDeclAttr.cpp
@@ -4506,7 +4506,7 @@
 break;
   case 7:
 if (Str == "pointer")
-  DestWidth = S.Context.getTargetInfo().getPointerWidth(0);
+  DestWidth = S.Context.getTargetInfo().getPointerWidth(LangAS::Default);
 break;
   case 11:
 if (Str == "unwind_word")
Index: clang/lib/Sema/SemaChecking.cpp

[PATCH] D135171: FreeBSD: enable __float128 on x86

[Alexander Richardson via Phabricator via cfe-commits]

arichardson added a comment.

@brooks do you want me to commit this for you or do you have commit access?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135171

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

[PATCH] D138722: Overload all llvm.annotation intrinsics for globals argument

[Alexander Richardson via Phabricator via cfe-commits]

arichardson created this revision.
arichardson added reviewers: arsenm, bader, Tyker, nikic.
Herald added subscribers: jrtc27, hiraditya.
Herald added a project: All.
arichardson requested review of this revision.
Herald added subscribers: llvm-commits, cfe-commits, jdoerfert, wdng.
Herald added projects: clang, LLVM.

The global constant arguments could be in a different address space
than the first argument, so we have to add another overloaded argument.
This patch was originally made for CHERI LLVM (where globals can be in
address space 200), but it also appears to be useful for in-tree targets
as can be seen from the test diffs.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D138722

Files:
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/CodeGen/CodeGenTypeCache.h
  clang/test/CodeGen/annotations-field.c
  clang/test/CodeGen/annotations-global.c
  clang/test/CodeGen/annotations-loc.c
  clang/test/CodeGen/annotations-var.c
  clang/test/CodeGenCXX/attr-annotate.cpp
  clang/test/CodeGenCXX/attr-annotate2.cpp
  clang/test/CodeGenSYCL/field-annotate-addr-space.cpp
  llvm/include/llvm/IR/Intrinsics.td
  llvm/lib/IR/AutoUpgrade.cpp
  llvm/test/Analysis/CostModel/X86/free-intrinsics.ll
  llvm/test/Analysis/CostModel/free-intrinsics-datalayout.ll
  llvm/test/Analysis/CostModel/free-intrinsics-no_info.ll
  llvm/test/Analysis/ScalarEvolution/annotation-intrinsics.ll
  llvm/test/Assembler/opaque-ptr-intrinsic-remangling.ll
  llvm/test/Bitcode/upgrade-annotation.ll
  llvm/test/Bitcode/upgrade-annotation.ll.bc
  llvm/test/Bitcode/upgrade-ptr-annotation.ll
  llvm/test/Bitcode/upgrade-var-annotation.ll
  llvm/test/Transforms/InstCombine/annotation-intrinsic.ll
  llvm/test/Transforms/InstCombine/assume_inevitable.ll

Index: llvm/test/Transforms/InstCombine/assume_inevitable.ll
===
--- llvm/test/Transforms/InstCombine/assume_inevitable.ll
+++ llvm/test/Transforms/InstCombine/assume_inevitable.ll
@@ -10,11 +10,11 @@
 ; CHECK-NEXT:[[M:%.*]] = alloca i64, align 8
 ; CHECK-NEXT:[[TMP0:%.*]] = load i32, ptr [[A:%.*]], align 32
 ; CHECK-NEXT:[[LOADRES:%.*]] = load i32, ptr [[B:%.*]], align 4
-; CHECK-NEXT:[[LOADRES2:%.*]] = call i32 @llvm.annotation.i32(i32 [[LOADRES]], ptr nonnull @.str, ptr nonnull @.str1, i32 2)
+; CHECK-NEXT:[[LOADRES2:%.*]] = call i32 @llvm.annotation.i32.p0(i32 [[LOADRES]], ptr nonnull @.str, ptr nonnull @.str1, i32 2)
 ; CHECK-NEXT:store i32 [[LOADRES2]], ptr [[A]], align 32
 ; CHECK-NEXT:[[DUMMY_EQ:%.*]] = icmp ugt i32 [[LOADRES]], 42
 ; CHECK-NEXT:tail call void @llvm.assume(i1 [[DUMMY_EQ]])
-; CHECK-NEXT:[[M_A:%.*]] = call ptr @llvm.ptr.annotation.p0(ptr nonnull [[M]], ptr nonnull @.str, ptr nonnull @.str1, i32 2, ptr null)
+; CHECK-NEXT:[[M_A:%.*]] = call ptr @llvm.ptr.annotation.p0.p0(ptr nonnull [[M]], ptr nonnull @.str, ptr nonnull @.str1, i32 2, ptr null)
 ; CHECK-NEXT:[[OBJSZ:%.*]] = call i64 @llvm.objectsize.i64.p0(ptr [[C:%.*]], i1 false, i1 false, i1 false)
 ; CHECK-NEXT:store i64 [[OBJSZ]], ptr [[M_A]], align 4
 ; CHECK-NEXT:[[PTRINT:%.*]] = ptrtoint ptr [[A]] to i64
Index: llvm/test/Transforms/InstCombine/annotation-intrinsic.ll
===
--- llvm/test/Transforms/InstCombine/annotation-intrinsic.ll
+++ llvm/test/Transforms/InstCombine/annotation-intrinsic.ll
@@ -12,7 +12,7 @@
 ; CHECK-LABEL: @annotated(
 ; CHECK-NEXT:  entry:
 ; CHECK-NEXT:[[TMP0:%.*]] = load i32, ptr [[C:%.*]], align 4
-; CHECK-NEXT:[[TMP1:%.*]] = call i32 @llvm.annotation.i32(i32 [[TMP0]], ptr undef, ptr undef, i32 undef)
+; CHECK-NEXT:[[TMP1:%.*]] = call i32 @llvm.annotation.i32.p0(i32 [[TMP0]], ptr undef, ptr undef, i32 undef)
 ; CHECK-NEXT:[[ADD:%.*]] = add nsw i32 [[TMP1]], [[TMP0]]
 ; CHECK-NEXT:ret i32 [[ADD]]
 ;
Index: llvm/test/Bitcode/upgrade-var-annotation.ll
===
--- llvm/test/Bitcode/upgrade-var-annotation.ll
+++ llvm/test/Bitcode/upgrade-var-annotation.ll
@@ -1,15 +1,16 @@
 ; Test upgrade of var.annotation intrinsics.
 ;
+; RUN: llvm-as < %s | llvm-dis | FileCheck %s
 ; RUN: llvm-dis < %s.bc | FileCheck %s
 
 
 define void @f(i8* %arg0, i8* %arg1, i8* %arg2, i32 %arg3) {
 ;CHECK: @f(i8* [[ARG0:%.*]], i8* [[ARG1:%.*]], i8* [[ARG2:%.*]], i32 [[ARG3:%.*]])
   call void @llvm.var.annotation(i8* %arg0, i8* %arg1, i8* %arg2, i32 %arg3)
-;CHECK:  call void @llvm.var.annotation(i8* [[ARG0]], i8* [[ARG1]], i8* [[ARG2]], i32 [[ARG3]], i8* null)
+;CHECK:  call void @llvm.var.annotation.p0i8.p0i8(i8* [[ARG0]], i8* [[ARG1]], i8* [[ARG2]], i32 [[ARG3]], i8* null)
   ret void
 }
 
 ; Function Attrs: nofree nosync nounwind willreturn
 declare void @llvm.var.annotation(i8*, i8*, i8*, i32)
-; CHECK: declare void @llvm.var.annotation(i8*, i8*, i8*, i32, i8*)
+; CHECK: declare 

[PATCH] D138681: [AVR] Fix broken bitcast for aliases in non-zero address space

[Alexander Richardson via Phabricator via cfe-commits]

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

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138681

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

[PATCH] D119541: [RISCV] Fix RISCVTargetInfo::initFeatureMap, add non-ISA features back after implication

[Alexander Richardson via Phabricator via cfe-commits]

arichardson added a comment.
Herald added subscribers: sunshaoce, StephenFan, shiva0217.
Herald added a project: All.

I just noticed that target features (e.g. -mrelax) are broken in all LLVM 14 
releases due to D113336  . This should have 
been cherry-picked back tot the release branch, but it's too late now. In the 
future please ensure that important fixes such as this one end up on the 
release branch as well.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119541

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

[PATCH] D138284: Fix incorrect cast in VisitSYCLUniqueStableNameExpr

[Alexander Richardson via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG0745b0c0354a: Fix incorrect cast in 
VisitSYCLUniqueStableNameExpr (authored by arichardson).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138284

Files:
  clang/lib/CodeGen/CGExprScalar.cpp
  clang/test/CodeGenSYCL/unique_stable_name.cpp
  clang/test/CodeGenSYCL/unique_stable_name_windows_diff.cpp

Index: clang/test/CodeGenSYCL/unique_stable_name_windows_diff.cpp
===
--- clang/test/CodeGenSYCL/unique_stable_name_windows_diff.cpp
+++ clang/test/CodeGenSYCL/unique_stable_name_windows_diff.cpp
@@ -1,5 +1,5 @@
-// RUN: %clang_cc1 -triple spir64-unknown-unknown-sycldevice -aux-triple x86_64-pc-windows-msvc -fsycl-is-device -disable-llvm-passes -fsycl-is-device -emit-llvm %s -o - | FileCheck %s
-// RUN: %clang_cc1 -triple x86_64-pc-windows-msvc -fsycl-is-device -disable-llvm-passes -fsycl-is-device -emit-llvm %s -o - | FileCheck %s
+// RUN: %clang_cc1 -triple spir64-unknown-unknown-sycldevice -aux-triple x86_64-pc-windows-msvc -fsycl-is-device -disable-llvm-passes -fsycl-is-device -emit-llvm %s -o - | FileCheck %s '-D$ADDRSPACE=addrspace(1) '
+// RUN: %clang_cc1 -triple x86_64-pc-windows-msvc -fsycl-is-device -disable-llvm-passes -fsycl-is-device -emit-llvm %s -o - | FileCheck %s '-D$ADDRSPACE='
 
 
 template
@@ -38,7 +38,7 @@
   // Make sure the following 3 are the same between the host and device compile.
   // Note that these are NOT the same value as eachother, they differ by the
   // signature.
-  // CHECK: private unnamed_addr constant [17 x i8] c"_ZTSZ4mainEUlvE_\00"
-  // CHECK: private unnamed_addr constant [17 x i8] c"_ZTSZ4mainEUliE_\00"
-  // CHECK: private unnamed_addr constant [17 x i8] c"_ZTSZ4mainEUldE_\00"
+  // CHECK: private unnamed_addr [[$ADDRSPACE]]constant [17 x i8] c"_ZTSZ4mainEUlvE_\00"
+  // CHECK: private unnamed_addr [[$ADDRSPACE]]constant [17 x i8] c"_ZTSZ4mainEUliE_\00"
+  // CHECK: private unnamed_addr [[$ADDRSPACE]]constant [17 x i8] c"_ZTSZ4mainEUldE_\00"
 }
Index: clang/test/CodeGenSYCL/unique_stable_name.cpp
===
--- clang/test/CodeGenSYCL/unique_stable_name.cpp
+++ clang/test/CodeGenSYCL/unique_stable_name.cpp
@@ -1,22 +1,22 @@
 // RUN: %clang_cc1 -no-opaque-pointers -triple spir64-unknown-unknown-sycldevice -fsycl-is-device -disable-llvm-passes -emit-llvm %s -o - | FileCheck %s
-// CHECK: @[[LAMBDA_KERNEL3:[^\w]+]] = private unnamed_addr constant [[LAMBDA_K3_SIZE:\[[0-9]+ x i8\]]] c"_ZTSZ4mainEUlPZ4mainEUlvE_E_\00"
-// CHECK: @[[INT1:[^\w]+]] = private unnamed_addr constant [[INT_SIZE:\[[0-9]+ x i8\]]] c"_ZTSi\00"
-// CHECK: @[[STRING:[^\w]+]] = private unnamed_addr constant [[STRING_SIZE:\[[0-9]+ x i8\]]] c"_ZTSAppL_ZZ4mainE1jE_i\00",
-// CHECK: @[[INT2:[^\w]+]] = private unnamed_addr constant [[INT_SIZE]] c"_ZTSi\00"
-// CHECK: @[[LAMBDA_X:[^\w]+]] = private unnamed_addr constant [[LAMBDA_X_SIZE:\[[0-9]+ x i8\]]] c"_ZTSZZ4mainENKUlvE0_clEvEUlvE_\00"
-// CHECK: @[[MACRO_X:[^\w]+]] = private unnamed_addr constant [[MACRO_SIZE:\[[0-9]+ x i8\]]] c"_ZTSZZ4mainENKUlvE0_clEvEUlvE0_\00"
-// CHECK: @[[MACRO_Y:[^\w]+]] =  private unnamed_addr constant [[MACRO_SIZE]] c"_ZTSZZ4mainENKUlvE0_clEvEUlvE1_\00"
-// CHECK: @{{.*}} = private unnamed_addr constant [32 x i8] c"_ZTSZZ4mainENKUlvE0_clEvEUlvE2_\00", align 1
-// CHECK: @{{.*}} = private unnamed_addr constant [32 x i8] c"_ZTSZZ4mainENKUlvE0_clEvEUlvE3_\00", align 1
-// CHECK: @[[MACRO_MACRO_X:[^\w]+]] = private unnamed_addr constant [[MACRO_MACRO_SIZE:\[[0-9]+ x i8\]]] c"_ZTSZZ4mainENKUlvE0_clEvEUlvE4_\00"
-// CHECK: @[[MACRO_MACRO_Y:[^\w]+]] = private unnamed_addr constant [[MACRO_MACRO_SIZE]] c"_ZTSZZ4mainENKUlvE0_clEvEUlvE5_\00"
-// CHECK: @[[INT3:[^\w]+]] = private unnamed_addr constant [[INT_SIZE]] c"_ZTSi\00"
-// CHECK: @[[LAMBDA:[^\w]+]] = private unnamed_addr constant [[LAMBDA_SIZE:\[[0-9]+ x i8\]]] c"_ZTSZZ4mainENKUlvE0_clEvEUlvE_\00"
-// CHECK: @[[LAMBDA_IN_DEP_INT:[^\w]+]] = private unnamed_addr constant [[DEP_INT_SIZE:\[[0-9]+ x i8\]]] c"_ZTSZ28lambda_in_dependent_functionIiEvvEUlvE_\00",
-// CHECK: @[[LAMBDA_IN_DEP_X:[^\w]+]] = private unnamed_addr constant [[DEP_LAMBDA_SIZE:\[[0-9]+ x i8\]]] c"_ZTSZ28lambda_in_dependent_functionIZZ4mainENKUlvE0_clEvEUlvE_EvvEUlvE_\00",
-// CHECK: @[[LAMBDA_NO_DEP:[^\w]+]] = private unnamed_addr constant [[NO_DEP_LAMBDA_SIZE:\[[0-9]+ x i8\]]] c"_ZTSZ13lambda_no_depIidEvT_T0_EUlidE_\00",
-// CHECK: @[[LAMBDA_TWO_DEP:[^\w]+]] = private unnamed_addr constant [[DEP_LAMBDA1_SIZE:\[[0-9]+ x i8\]]] c"_ZTSZ14lambda_two_depIZZ4mainENKUlvE0_clEvEUliE_ZZ4mainENKS0_clEvEUldE_EvvEUlvE_\00",
-// CHECK: @[[LAMBDA_TWO_DEP2:[^\w]+]] = private unnamed_addr constant [[DEP_LAMBDA2_SIZE:\[[0-9]+ x i8\]]] c"_ZTSZ14lambda_two_depIZZ4mainENKUlvE0_clEvEUldE_ZZ4mainENKS0_clEvEUliE_EvvEUlvE_\00",
+// CHECK: 

[PATCH] D138316: [ASTContext] Avoid duplicating address space map. NFCI

[Alexander Richardson via Phabricator via cfe-commits]

arichardson created this revision.
arichardson added reviewers: clang, pcc.
Herald added a project: All.
arichardson requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

ASTContext was holding onto a pointer to the Clang->LLVM address space map
which is stored inside TargetInfo. Instead of doing this, we can forward to
TargetInfo instead. This change will allow us to eventually remove
getTargetAddressSpace() from ASTContext and only have this information in
TargetInfo.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D138316

Files:
  clang/include/clang/AST/ASTContext.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/Basic/TargetInfo.cpp

Index: clang/lib/Basic/TargetInfo.cpp
===
--- clang/lib/Basic/TargetInfo.cpp
+++ clang/lib/Basic/TargetInfo.cpp
@@ -24,6 +24,30 @@
 using namespace clang;
 
 static const LangASMap DefaultAddrSpaceMap = {0};
+// The fake address space map must have a distinct entry for each
+// language-specific address space.
+static const LangASMap FakeAddrSpaceMap = {
+0,  // Default
+1,  // opencl_global
+3,  // opencl_local
+2,  // opencl_constant
+0,  // opencl_private
+4,  // opencl_generic
+5,  // opencl_global_device
+6,  // opencl_global_host
+7,  // cuda_device
+8,  // cuda_constant
+9,  // cuda_shared
+1,  // sycl_global
+5,  // sycl_global_device
+6,  // sycl_global_host
+3,  // sycl_local
+0,  // sycl_private
+10, // ptr32_sptr
+11, // ptr32_uptr
+12, // ptr64
+13, // hlsl_groupshared
+};
 
 // TargetInfo Constructor.
 TargetInfo::TargetInfo(const llvm::Triple ) : Triple(T) {
@@ -487,6 +511,9 @@
 
   if (Opts.MaxBitIntWidth)
 MaxBitIntWidth = Opts.MaxBitIntWidth;
+
+  if (Opts.FakeAddressSpaceMap)
+AddrSpaceMap = 
 }
 
 bool TargetInfo::initFeatureMap(
Index: clang/lib/AST/ASTContext.cpp
===
--- clang/lib/AST/ASTContext.cpp
+++ clang/lib/AST/ASTContext.cpp
@@ -929,39 +929,6 @@
   return *ParentMapCtx.get();
 }
 
-static const LangASMap *getAddressSpaceMap(const TargetInfo ,
-   const LangOptions ) {
-  if (LOpts.FakeAddressSpaceMap) {
-// The fake address space map must have a distinct entry for each
-// language-specific address space.
-static const unsigned FakeAddrSpaceMap[] = {
-0,  // Default
-1,  // opencl_global
-3,  // opencl_local
-2,  // opencl_constant
-0,  // opencl_private
-4,  // opencl_generic
-5,  // opencl_global_device
-6,  // opencl_global_host
-7,  // cuda_device
-8,  // cuda_constant
-9,  // cuda_shared
-1,  // sycl_global
-5,  // sycl_global_device
-6,  // sycl_global_host
-3,  // sycl_local
-0,  // sycl_private
-10, // ptr32_sptr
-11, // ptr32_uptr
-12, // ptr64
-13, // hlsl_groupshared
-};
-return 
-  } else {
-return ();
-  }
-}
-
 static bool isAddrSpaceMapManglingEnabled(const TargetInfo ,
   const LangOptions ) {
   switch (LangOpts.getAddressSpaceMapMangling()) {
@@ -1292,7 +1259,6 @@
   this->AuxTarget = AuxTarget;
 
   ABI.reset(createCXXABI(Target));
-  AddrSpaceMap = getAddressSpaceMap(Target, LangOpts);
   AddrSpaceMapMangling = isAddrSpaceMapManglingEnabled(Target, LangOpts);
 
   // C99 6.2.5p19.
@@ -12228,10 +12194,7 @@
 }
 
 unsigned ASTContext::getTargetAddressSpace(LangAS AS) const {
-  if (isTargetAddressSpace(AS))
-return toTargetAddressSpace(AS);
-  else
-return (*AddrSpaceMap)[(unsigned)AS];
+  return getTargetInfo().getTargetAddressSpace(AS);
 }
 
 bool ASTContext::hasSameExpr(const Expr *X, const Expr *Y) const {
Index: clang/include/clang/AST/ASTContext.h
===
--- clang/include/clang/AST/ASTContext.h
+++ clang/include/clang/AST/ASTContext.h
@@ -640,9 +640,6 @@
   std::unique_ptr ABI;
   CXXABI *createCXXABI(const TargetInfo );
 
-  /// The logical -> physical address space map.
-  const LangASMap *AddrSpaceMap = nullptr;
-
   /// Address space map mangling must be used with language specific
   /// address spaces (e.g. OpenCL/CUDA)
   bool AddrSpaceMapMangling;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D138296: [clang] Avoid duplicating ProgramAddressSpace in TargetInfo. NFCI

[Alexander Richardson via Phabricator via cfe-commits]

arichardson added a comment.

In D138296#3937224 , @eandrews wrote:

> Functionally this looks ok to me. However I am not sure if CodeGenTypes is 
> the 'right' place for this function to live, considering that other functions 
> with similar functionality are in ASTContext - including overloads of 
> getTargetAddressSpace( ). @erichkeane @aaron.ballman could you please take a 
> look?

My view is that the parts that interact with LLVM IR should really live in 
CodeGen/ and not Basic/ or AST/. I will see how difficult it would be to move 
the remaining target (LLVM IR) address space handling code to CodeGen/


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138296

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

[PATCH] D138296: [clang] Avoid duplicating ProgramAddressSpace in TargetInfo. NFCI

[Alexander Richardson via Phabricator via cfe-commits]

arichardson updated this revision to Diff 476473.
arichardson added a comment.

clang-format


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138296

Files:
  clang/include/clang/AST/ASTContext.h
  clang/include/clang/Basic/TargetInfo.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/Basic/TargetInfo.cpp
  clang/lib/Basic/Targets/AVR.h
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/lib/CodeGen/CGDeclCXX.cpp
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/CodeGen/CodeGenTypes.cpp
  clang/lib/CodeGen/CodeGenTypes.h

Index: clang/lib/CodeGen/CodeGenTypes.h
===
--- clang/lib/CodeGen/CodeGenTypes.h
+++ clang/lib/CodeGen/CodeGenTypes.h
@@ -305,7 +305,7 @@
   bool isRecordBeingLaidOut(const Type *Ty) const {
 return RecordsBeingLaidOut.count(Ty);
   }
-
+  unsigned getTargetAddressSpace(QualType T) const;
 };
 
 }  // end namespace CodeGen
Index: clang/lib/CodeGen/CodeGenTypes.cpp
===
--- clang/lib/CodeGen/CodeGenTypes.cpp
+++ clang/lib/CodeGen/CodeGenTypes.cpp
@@ -655,7 +655,7 @@
 const ReferenceType *RTy = cast(Ty);
 QualType ETy = RTy->getPointeeType();
 llvm::Type *PointeeType = ConvertTypeForMem(ETy);
-unsigned AS = Context.getTargetAddressSpace(ETy);
+unsigned AS = getTargetAddressSpace(ETy);
 ResultType = llvm::PointerType::get(PointeeType, AS);
 break;
   }
@@ -665,7 +665,7 @@
 llvm::Type *PointeeType = ConvertTypeForMem(ETy);
 if (PointeeType->isVoidTy())
   PointeeType = llvm::Type::getInt8Ty(getLLVMContext());
-unsigned AS = Context.getTargetAddressSpace(ETy);
+unsigned AS = getTargetAddressSpace(ETy);
 ResultType = llvm::PointerType::get(PointeeType, AS);
 break;
   }
@@ -958,3 +958,13 @@
 bool CodeGenTypes::isZeroInitializable(const RecordDecl *RD) {
   return getCGRecordLayout(RD).isZeroInitializable();
 }
+
+unsigned CodeGenTypes::getTargetAddressSpace(QualType T) const {
+  // Return the address space for the type. If the type is a
+  // function type without an address space qualifier, the
+  // program address space is used. Otherwise, the target picks
+  // the best address space based on the type information
+  return T->isFunctionType() && !T.hasAddressSpace()
+ ? getDataLayout().getProgramAddressSpace()
+ : getContext().getTargetAddressSpace(T.getAddressSpace());
+}
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -3196,7 +3196,7 @@
   // See if there is already something with the target's name in the module.
   llvm::GlobalValue *Entry = GetGlobalValue(AA->getAliasee());
   if (Entry) {
-unsigned AS = getContext().getTargetAddressSpace(VD->getType());
+unsigned AS = getTypes().getTargetAddressSpace(VD->getType());
 auto Ptr = llvm::ConstantExpr::getBitCast(Entry, DeclTy->getPointerTo(AS));
 return ConstantAddress(Ptr, DeclTy, Alignment);
   }
@@ -3759,7 +3759,7 @@
   if (getTarget().supportsIFunc()) {
 ResolverType = llvm::FunctionType::get(
 llvm::PointerType::get(DeclTy,
-   Context.getTargetAddressSpace(FD->getType())),
+   getTypes().getTargetAddressSpace(FD->getType())),
 false);
   }
   else {
@@ -3897,8 +3897,8 @@
   // cpu_dispatch will be emitted in this translation unit.
   if (getTarget().supportsIFunc() && !FD->isCPUSpecificMultiVersion()) {
 llvm::Type *ResolverType = llvm::FunctionType::get(
-llvm::PointerType::get(
-DeclTy, getContext().getTargetAddressSpace(FD->getType())),
+llvm::PointerType::get(DeclTy,
+   getTypes().getTargetAddressSpace(FD->getType())),
 false);
 llvm::Constant *Resolver = GetOrCreateLLVMFunction(
 MangledName + ".resolver", ResolverType, GlobalDecl{},
Index: clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp
===
--- clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp
+++ clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp
@@ -3128,7 +3128,7 @@
   const Type *NonQualTy = QC.strip(NativeParamType);
   QualType NativePointeeTy = cast(NonQualTy)->getPointeeType();
   unsigned NativePointeeAddrSpace =
-  CGF.getContext().getTargetAddressSpace(NativePointeeTy);
+  CGF.getTypes().getTargetAddressSpace(NativePointeeTy);
   QualType TargetTy = TargetParam->getType();
   llvm::Value *TargetAddr = CGF.EmitLoadOfScalar(
   LocalAddr, /*Volatile=*/false, TargetTy, SourceLocation());
Index: clang/lib/CodeGen/CGExpr.cpp
===
--- clang/lib/CodeGen/CGExpr.cpp
+++ 

[PATCH] D138295: [clang][TargetInfo] Use LangAS for getPointer{Width,Align}()

[Alexander Richardson via Phabricator via cfe-commits]

arichardson updated this revision to Diff 476472.
arichardson added a comment.

clang-format

I could also use {} instead of `LangAS::Default`, but the latter seems more 
obvious


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138295

Files:
  clang/include/clang/Basic/TargetInfo.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/AST/ItaniumCXXABI.cpp
  clang/lib/AST/Mangle.cpp
  clang/lib/AST/MicrosoftCXXABI.cpp
  clang/lib/AST/MicrosoftMangle.cpp
  clang/lib/AST/RecordLayoutBuilder.cpp
  clang/lib/AST/VTableBuilder.cpp
  clang/lib/Basic/Targets/AMDGPU.cpp
  clang/lib/Basic/Targets/AMDGPU.h
  clang/lib/Basic/Targets/Mips.cpp
  clang/lib/Basic/Targets/NVPTX.cpp
  clang/lib/Basic/Targets/X86.h
  clang/lib/CodeGen/CGBlocks.cpp
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/lib/CodeGen/CGExprAgg.cpp
  clang/lib/CodeGen/CGObjCGNU.cpp
  clang/lib/CodeGen/CGObjCMac.cpp
  clang/lib/CodeGen/CGVTables.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/CodeGen/ItaniumCXXABI.cpp
  clang/lib/CodeGen/MicrosoftCXXABI.cpp
  clang/lib/CodeGen/PatternInit.cpp
  clang/lib/CodeGen/SwiftCallingConv.cpp
  clang/lib/CodeGen/TargetInfo.cpp
  clang/lib/Frontend/InitPreprocessor.cpp
  clang/lib/Sema/SemaChecking.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/lib/Sema/SemaType.cpp
  clang/lib/StaticAnalyzer/Checkers/ObjCContainersASTChecker.cpp
  clang/unittests/CodeGen/TestCompiler.h

Index: clang/unittests/CodeGen/TestCompiler.h
===
--- clang/unittests/CodeGen/TestCompiler.h
+++ clang/unittests/CodeGen/TestCompiler.h
@@ -48,7 +48,7 @@
 std::make_shared(compiler.getTargetOpts(;
 
 const clang::TargetInfo  = compiler.getTarget();
-PtrSize = TInfo.getPointerWidth(0) / 8;
+PtrSize = TInfo.getPointerWidth(clang::LangAS::Default) / 8;
 
 compiler.createFileManager();
 compiler.createSourceManager(compiler.getFileManager());
Index: clang/lib/StaticAnalyzer/Checkers/ObjCContainersASTChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/ObjCContainersASTChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/ObjCContainersASTChecker.cpp
@@ -72,7 +72,7 @@
 public:
   WalkAST(BugReporter , const CheckerBase *checker, AnalysisDeclContext *ac)
   : BR(br), Checker(checker), AC(ac), ASTC(AC->getASTContext()),
-PtrWidth(ASTC.getTargetInfo().getPointerWidth(0)) {}
+PtrWidth(ASTC.getTargetInfo().getPointerWidth(LangAS::Default)) {}
 
   // Statement visitor methods.
   void VisitChildren(Stmt *S);
Index: clang/lib/Sema/SemaType.cpp
===
--- clang/lib/Sema/SemaType.cpp
+++ clang/lib/Sema/SemaType.cpp
@@ -7288,7 +7288,8 @@
 
   // Add address space to type based on its attributes.
   LangAS ASIdx = LangAS::Default;
-  uint64_t PtrWidth = S.Context.getTargetInfo().getPointerWidth(0);
+  uint64_t PtrWidth =
+  S.Context.getTargetInfo().getPointerWidth(LangAS::Default);
   if (PtrWidth == 32) {
 if (Attrs[attr::Ptr64])
   ASIdx = LangAS::ptr64;
Index: clang/lib/Sema/SemaExprCXX.cpp
===
--- clang/lib/Sema/SemaExprCXX.cpp
+++ clang/lib/Sema/SemaExprCXX.cpp
@@ -2661,7 +2661,9 @@
   // tree? Or should the consumer just recalculate the value?
   // FIXME: Using a dummy value will interact poorly with attribute enable_if.
   IntegerLiteral Size(
-  Context, llvm::APInt::getZero(Context.getTargetInfo().getPointerWidth(0)),
+  Context,
+  llvm::APInt::getZero(
+  Context.getTargetInfo().getPointerWidth(LangAS::Default)),
   Context.getSizeType(), SourceLocation());
   AllocArgs.push_back();
 
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -16720,7 +16720,7 @@
   // The type of __null will be int or long, depending on the size of
   // pointers on the target.
   QualType Ty;
-  unsigned pw = Context.getTargetInfo().getPointerWidth(0);
+  unsigned pw = Context.getTargetInfo().getPointerWidth(LangAS::Default);
   if (pw == Context.getTargetInfo().getIntWidth())
 Ty = Context.IntTy;
   else if (pw == Context.getTargetInfo().getLongWidth())
Index: clang/lib/Sema/SemaDeclAttr.cpp
===
--- clang/lib/Sema/SemaDeclAttr.cpp
+++ clang/lib/Sema/SemaDeclAttr.cpp
@@ -4506,7 +4506,7 @@
 break;
   case 7:
 if (Str == "pointer")
-  DestWidth = S.Context.getTargetInfo().getPointerWidth(0);
+  DestWidth = S.Context.getTargetInfo().getPointerWidth(LangAS::Default);
 break;
   case 11:
 if (Str == "unwind_word")
Index: clang/lib/Sema/SemaChecking.cpp

[PATCH] D138284: Fix incorrect cast in VisitSYCLUniqueStableNameExpr

[Alexander Richardson via Phabricator via cfe-commits]

arichardson updated this revision to Diff 476470.
arichardson added a comment.

Fix to use addrspace(1)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138284

Files:
  clang/lib/CodeGen/CGExprScalar.cpp
  clang/test/CodeGenSYCL/unique_stable_name.cpp
  clang/test/CodeGenSYCL/unique_stable_name_windows_diff.cpp

Index: clang/test/CodeGenSYCL/unique_stable_name_windows_diff.cpp
===
--- clang/test/CodeGenSYCL/unique_stable_name_windows_diff.cpp
+++ clang/test/CodeGenSYCL/unique_stable_name_windows_diff.cpp
@@ -1,5 +1,5 @@
-// RUN: %clang_cc1 -triple spir64-unknown-unknown-sycldevice -aux-triple x86_64-pc-windows-msvc -fsycl-is-device -disable-llvm-passes -fsycl-is-device -emit-llvm %s -o - | FileCheck %s
-// RUN: %clang_cc1 -triple x86_64-pc-windows-msvc -fsycl-is-device -disable-llvm-passes -fsycl-is-device -emit-llvm %s -o - | FileCheck %s
+// RUN: %clang_cc1 -triple spir64-unknown-unknown-sycldevice -aux-triple x86_64-pc-windows-msvc -fsycl-is-device -disable-llvm-passes -fsycl-is-device -emit-llvm %s -o - | FileCheck %s '-D$ADDRSPACE=addrspace(1) '
+// RUN: %clang_cc1 -triple x86_64-pc-windows-msvc -fsycl-is-device -disable-llvm-passes -fsycl-is-device -emit-llvm %s -o - | FileCheck %s '-D$ADDRSPACE='
 
 
 template
@@ -38,7 +38,7 @@
   // Make sure the following 3 are the same between the host and device compile.
   // Note that these are NOT the same value as eachother, they differ by the
   // signature.
-  // CHECK: private unnamed_addr constant [17 x i8] c"_ZTSZ4mainEUlvE_\00"
-  // CHECK: private unnamed_addr constant [17 x i8] c"_ZTSZ4mainEUliE_\00"
-  // CHECK: private unnamed_addr constant [17 x i8] c"_ZTSZ4mainEUldE_\00"
+  // CHECK: private unnamed_addr [[$ADDRSPACE]]constant [17 x i8] c"_ZTSZ4mainEUlvE_\00"
+  // CHECK: private unnamed_addr [[$ADDRSPACE]]constant [17 x i8] c"_ZTSZ4mainEUliE_\00"
+  // CHECK: private unnamed_addr [[$ADDRSPACE]]constant [17 x i8] c"_ZTSZ4mainEUldE_\00"
 }
Index: clang/test/CodeGenSYCL/unique_stable_name.cpp
===
--- clang/test/CodeGenSYCL/unique_stable_name.cpp
+++ clang/test/CodeGenSYCL/unique_stable_name.cpp
@@ -1,22 +1,22 @@
 // RUN: %clang_cc1 -no-opaque-pointers -triple spir64-unknown-unknown-sycldevice -fsycl-is-device -disable-llvm-passes -emit-llvm %s -o - | FileCheck %s
-// CHECK: @[[LAMBDA_KERNEL3:[^\w]+]] = private unnamed_addr constant [[LAMBDA_K3_SIZE:\[[0-9]+ x i8\]]] c"_ZTSZ4mainEUlPZ4mainEUlvE_E_\00"
-// CHECK: @[[INT1:[^\w]+]] = private unnamed_addr constant [[INT_SIZE:\[[0-9]+ x i8\]]] c"_ZTSi\00"
-// CHECK: @[[STRING:[^\w]+]] = private unnamed_addr constant [[STRING_SIZE:\[[0-9]+ x i8\]]] c"_ZTSAppL_ZZ4mainE1jE_i\00",
-// CHECK: @[[INT2:[^\w]+]] = private unnamed_addr constant [[INT_SIZE]] c"_ZTSi\00"
-// CHECK: @[[LAMBDA_X:[^\w]+]] = private unnamed_addr constant [[LAMBDA_X_SIZE:\[[0-9]+ x i8\]]] c"_ZTSZZ4mainENKUlvE0_clEvEUlvE_\00"
-// CHECK: @[[MACRO_X:[^\w]+]] = private unnamed_addr constant [[MACRO_SIZE:\[[0-9]+ x i8\]]] c"_ZTSZZ4mainENKUlvE0_clEvEUlvE0_\00"
-// CHECK: @[[MACRO_Y:[^\w]+]] =  private unnamed_addr constant [[MACRO_SIZE]] c"_ZTSZZ4mainENKUlvE0_clEvEUlvE1_\00"
-// CHECK: @{{.*}} = private unnamed_addr constant [32 x i8] c"_ZTSZZ4mainENKUlvE0_clEvEUlvE2_\00", align 1
-// CHECK: @{{.*}} = private unnamed_addr constant [32 x i8] c"_ZTSZZ4mainENKUlvE0_clEvEUlvE3_\00", align 1
-// CHECK: @[[MACRO_MACRO_X:[^\w]+]] = private unnamed_addr constant [[MACRO_MACRO_SIZE:\[[0-9]+ x i8\]]] c"_ZTSZZ4mainENKUlvE0_clEvEUlvE4_\00"
-// CHECK: @[[MACRO_MACRO_Y:[^\w]+]] = private unnamed_addr constant [[MACRO_MACRO_SIZE]] c"_ZTSZZ4mainENKUlvE0_clEvEUlvE5_\00"
-// CHECK: @[[INT3:[^\w]+]] = private unnamed_addr constant [[INT_SIZE]] c"_ZTSi\00"
-// CHECK: @[[LAMBDA:[^\w]+]] = private unnamed_addr constant [[LAMBDA_SIZE:\[[0-9]+ x i8\]]] c"_ZTSZZ4mainENKUlvE0_clEvEUlvE_\00"
-// CHECK: @[[LAMBDA_IN_DEP_INT:[^\w]+]] = private unnamed_addr constant [[DEP_INT_SIZE:\[[0-9]+ x i8\]]] c"_ZTSZ28lambda_in_dependent_functionIiEvvEUlvE_\00",
-// CHECK: @[[LAMBDA_IN_DEP_X:[^\w]+]] = private unnamed_addr constant [[DEP_LAMBDA_SIZE:\[[0-9]+ x i8\]]] c"_ZTSZ28lambda_in_dependent_functionIZZ4mainENKUlvE0_clEvEUlvE_EvvEUlvE_\00",
-// CHECK: @[[LAMBDA_NO_DEP:[^\w]+]] = private unnamed_addr constant [[NO_DEP_LAMBDA_SIZE:\[[0-9]+ x i8\]]] c"_ZTSZ13lambda_no_depIidEvT_T0_EUlidE_\00",
-// CHECK: @[[LAMBDA_TWO_DEP:[^\w]+]] = private unnamed_addr constant [[DEP_LAMBDA1_SIZE:\[[0-9]+ x i8\]]] c"_ZTSZ14lambda_two_depIZZ4mainENKUlvE0_clEvEUliE_ZZ4mainENKS0_clEvEUldE_EvvEUlvE_\00",
-// CHECK: @[[LAMBDA_TWO_DEP2:[^\w]+]] = private unnamed_addr constant [[DEP_LAMBDA2_SIZE:\[[0-9]+ x i8\]]] c"_ZTSZ14lambda_two_depIZZ4mainENKUlvE0_clEvEUldE_ZZ4mainENKS0_clEvEUliE_EvvEUlvE_\00",
+// CHECK: @[[LAMBDA_KERNEL3:[^\w]+]] = private unnamed_addr addrspace(1) constant 

[PATCH] D138284: Fix incorrect cast in VisitSYCLUniqueStableNameExpr

[Alexander Richardson via Phabricator via cfe-commits]

arichardson added inline comments.



Comment at: clang/lib/CodeGen/CGExprScalar.cpp:1635
+  Context.getTargetInfo().getConstantAddressSpace().value_or(
+  LangAS::Default));
   llvm::Constant *GlobalConstStr = Builder.CreateGlobalStringPtr(

arichardson wrote:
> bader wrote:
> > > This changes the code generation for spir64 to place the globals in 
> > > addrspace(4). I believe is correct, but it would be good for someone who 
> > > is familiar with the target to confirm.
> > 
> > Globals must reside in `sycl_global` namespace, which is `addrspace(1)` for 
> > spir* targets.
> > `addrspace(4)` represents "generic" address space, which is a placeholder 
> > for a specific address space. If we leave it `addrspace(4)` for global 
> > definition, the compiler won't be able to infer genuine address space.
> Okay that's interesting - I guess it means we should not be using 
> `getConstantAddressSpace()` here? Or getConstantAddressSpace() should 
> actually return a value that maps to `addrspace(1)`?
Ah it looks like we should be using 
`CodeGenModule::GetGlobalConstantAddressSpace()` instead of 
`getTarget().getConstantAddressSpace()`. Is that correct?


```
LangAS CodeGenModule::GetGlobalConstantAddressSpace() const {
  // OpenCL v1.2 s6.5.3: a string literal is in the constant address space.
  if (LangOpts.OpenCL)
return LangAS::opencl_constant;
  if (LangOpts.SYCLIsDevice)
return LangAS::sycl_global;
  if (LangOpts.HIP && LangOpts.CUDAIsDevice && getTriple().isSPIRV())
// For HIPSPV map literals to cuda_device (maps to CrossWorkGroup in SPIR-V)
// instead of default AS (maps to Generic in SPIR-V). Otherwise, we end up
// with OpVariable instructions with Generic storage class which is not
// allowed (SPIR-V V1.6 s3.42.8). Also, mapping literals to SPIR-V
// UniformConstant storage class is not viable as pointers to it may not be
// casted to Generic pointers which are used to model HIP's "flat" pointers.
return LangAS::cuda_device;
  if (auto AS = getTarget().getConstantAddressSpace())
return *AS;
  return LangAS::Default;
}
```

Another problem appears to be that the default implementation of 
getConstantAddressSpace() returns `LangAS::Default` instead of None, so the 
.value_or() will never be used.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138284

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

[PATCH] D138284: Fix incorrect cast in VisitSYCLUniqueStableNameExpr

[Alexander Richardson via Phabricator via cfe-commits]

arichardson added a comment.

In D138284#3936846 , @erichkeane 
wrote:

> In D138284#3936830 , @aaron.ballman 
> wrote:
>
>> Adding @bader as SYCL code owner. The changes look reasonable to me, but 
>> codegen is not my area of expertise.
>
> Yeah, same to me.  I wrote this originally, but I think the address-space 
> stuff was added in the downstream by someone else (though I can't find proof 
> of it now!).

(Unfortunately) that was me. Not the best design, but it catches lots of subtle 
run-time errors at compile time. Has been extremely useful for the downstream 
CHERI fork.




Comment at: clang/lib/CodeGen/CGExprScalar.cpp:1635
+  Context.getTargetInfo().getConstantAddressSpace().value_or(
+  LangAS::Default));
   llvm::Constant *GlobalConstStr = Builder.CreateGlobalStringPtr(

bader wrote:
> > This changes the code generation for spir64 to place the globals in 
> > addrspace(4). I believe is correct, but it would be good for someone who is 
> > familiar with the target to confirm.
> 
> Globals must reside in `sycl_global` namespace, which is `addrspace(1)` for 
> spir* targets.
> `addrspace(4)` represents "generic" address space, which is a placeholder for 
> a specific address space. If we leave it `addrspace(4)` for global 
> definition, the compiler won't be able to infer genuine address space.
Okay that's interesting - I guess it means we should not be using 
`getConstantAddressSpace()` here? Or getConstantAddressSpace() should actually 
return a value that maps to `addrspace(1)`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138284

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

[PATCH] D138296: [clang] Avoid duplicating ProgramAddressSpace in TargetInfo. NFCI

[Alexander Richardson via Phabricator via cfe-commits]

arichardson created this revision.
arichardson added reviewers: eandrews, dylanmckay, bader, rjmccall.
Herald added subscribers: jrtc27, luismarques, s.egerton, Jim, PkmX, atanasyan, 
simoncook, kristof.beyls, sdardis.
Herald added a project: All.
arichardson requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This value was added to clang/Basic in D111566 
, but is only used during
codegen, where we can use the LLVM IR DataLayout instead. I noticed this
because the downstream CHERI targets would have to also set this value
for AArch64/RISC-V/MIPS. Instead of duplicating more information between
LLVM IR and Clang, this patch moves getTargetAddressSpace(QualType T) to
CodeGenTypes, where we can consult the DataLayout.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D138296

Files:
  clang/include/clang/AST/ASTContext.h
  clang/include/clang/Basic/TargetInfo.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/Basic/TargetInfo.cpp
  clang/lib/Basic/Targets/AVR.h
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/lib/CodeGen/CGDeclCXX.cpp
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/CodeGen/CodeGenTypes.cpp
  clang/lib/CodeGen/CodeGenTypes.h

Index: clang/lib/CodeGen/CodeGenTypes.h
===
--- clang/lib/CodeGen/CodeGenTypes.h
+++ clang/lib/CodeGen/CodeGenTypes.h
@@ -305,7 +305,7 @@
   bool isRecordBeingLaidOut(const Type *Ty) const {
 return RecordsBeingLaidOut.count(Ty);
   }
-
+  unsigned getTargetAddressSpace(QualType T) const;
 };
 
 }  // end namespace CodeGen
Index: clang/lib/CodeGen/CodeGenTypes.cpp
===
--- clang/lib/CodeGen/CodeGenTypes.cpp
+++ clang/lib/CodeGen/CodeGenTypes.cpp
@@ -655,7 +655,7 @@
 const ReferenceType *RTy = cast(Ty);
 QualType ETy = RTy->getPointeeType();
 llvm::Type *PointeeType = ConvertTypeForMem(ETy);
-unsigned AS = Context.getTargetAddressSpace(ETy);
+unsigned AS = getTargetAddressSpace(ETy);
 ResultType = llvm::PointerType::get(PointeeType, AS);
 break;
   }
@@ -665,7 +665,7 @@
 llvm::Type *PointeeType = ConvertTypeForMem(ETy);
 if (PointeeType->isVoidTy())
   PointeeType = llvm::Type::getInt8Ty(getLLVMContext());
-unsigned AS = Context.getTargetAddressSpace(ETy);
+unsigned AS = getTargetAddressSpace(ETy);
 ResultType = llvm::PointerType::get(PointeeType, AS);
 break;
   }
@@ -958,3 +958,13 @@
 bool CodeGenTypes::isZeroInitializable(const RecordDecl *RD) {
   return getCGRecordLayout(RD).isZeroInitializable();
 }
+
+unsigned CodeGenTypes::getTargetAddressSpace(QualType T) const {
+  // Return the address space for the type. If the type is a
+  // function type without an address space qualifier, the
+  // program address space is used. Otherwise, the target picks
+  // the best address space based on the type information
+  return T->isFunctionType() && !T.hasAddressSpace()
+ ? getDataLayout().getProgramAddressSpace()
+ : getContext().getTargetAddressSpace(T.getAddressSpace());
+}
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -3195,7 +3195,7 @@
   // See if there is already something with the target's name in the module.
   llvm::GlobalValue *Entry = GetGlobalValue(AA->getAliasee());
   if (Entry) {
-unsigned AS = getContext().getTargetAddressSpace(VD->getType());
+unsigned AS = getTypes().getTargetAddressSpace(VD->getType());
 auto Ptr = llvm::ConstantExpr::getBitCast(Entry, DeclTy->getPointerTo(AS));
 return ConstantAddress(Ptr, DeclTy, Alignment);
   }
@@ -3758,7 +3758,7 @@
   if (getTarget().supportsIFunc()) {
 ResolverType = llvm::FunctionType::get(
 llvm::PointerType::get(DeclTy,
-   Context.getTargetAddressSpace(FD->getType())),
+   getTypes().getTargetAddressSpace(FD->getType())),
 false);
   }
   else {
@@ -3897,7 +3897,7 @@
   if (getTarget().supportsIFunc() && !FD->isCPUSpecificMultiVersion()) {
 llvm::Type *ResolverType = llvm::FunctionType::get(
 llvm::PointerType::get(
-DeclTy, getContext().getTargetAddressSpace(FD->getType())),
+DeclTy, getTypes().getTargetAddressSpace(FD->getType())),
 false);
 llvm::Constant *Resolver = GetOrCreateLLVMFunction(
 MangledName + ".resolver", ResolverType, GlobalDecl{},
Index: clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp
===
--- clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp
+++ clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp
@@ -3128,7 +3128,7 @@
   const Type *NonQualTy = 

[PATCH] D138295: [clang][TargetInfo] Use LangAS for getPointer{Width,Align}()

[Alexander Richardson via Phabricator via cfe-commits]

arichardson created this revision.
arichardson added a reviewer: clang.
Herald added subscribers: steakhal, kosarev, mattd, gchakrabarti, asavonic, 
martong, kerbowa, atanasyan, jrtc27, jvesely, sdardis.
Herald added a reviewer: aaron.ballman.
Herald added a reviewer: NoQ.
Herald added a project: All.
arichardson requested review of this revision.
Herald added subscribers: cfe-commits, jholewinski.
Herald added a project: clang.

Mixing LLVM and Clang address spaces can result in subtle bugs, and there
is no need for this hook to use the LLVM IR level address spaces.
Most of this change is just replacing zero with LangAS::Default,
but it also allows us to remove a few calls to getTargetAddressSpace().

This also removes a stale comment+workaround in
CGDebugInfo::CreatePointerLikeType(): ASTContext::getTypeSize() does
return the expected size for ReferenceType (and handles address spaces).


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D138295

Files:
  clang/include/clang/Basic/TargetInfo.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/AST/ItaniumCXXABI.cpp
  clang/lib/AST/Mangle.cpp
  clang/lib/AST/MicrosoftCXXABI.cpp
  clang/lib/AST/MicrosoftMangle.cpp
  clang/lib/AST/RecordLayoutBuilder.cpp
  clang/lib/AST/VTableBuilder.cpp
  clang/lib/Basic/Targets/AMDGPU.cpp
  clang/lib/Basic/Targets/AMDGPU.h
  clang/lib/Basic/Targets/Mips.cpp
  clang/lib/Basic/Targets/NVPTX.cpp
  clang/lib/Basic/Targets/X86.h
  clang/lib/CodeGen/CGBlocks.cpp
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/lib/CodeGen/CGExprAgg.cpp
  clang/lib/CodeGen/CGObjCGNU.cpp
  clang/lib/CodeGen/CGObjCMac.cpp
  clang/lib/CodeGen/CGVTables.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/CodeGen/ItaniumCXXABI.cpp
  clang/lib/CodeGen/MicrosoftCXXABI.cpp
  clang/lib/CodeGen/PatternInit.cpp
  clang/lib/CodeGen/SwiftCallingConv.cpp
  clang/lib/CodeGen/TargetInfo.cpp
  clang/lib/Frontend/InitPreprocessor.cpp
  clang/lib/Sema/SemaChecking.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/lib/Sema/SemaType.cpp
  clang/lib/StaticAnalyzer/Checkers/ObjCContainersASTChecker.cpp
  clang/unittests/CodeGen/TestCompiler.h

Index: clang/unittests/CodeGen/TestCompiler.h
===
--- clang/unittests/CodeGen/TestCompiler.h
+++ clang/unittests/CodeGen/TestCompiler.h
@@ -48,7 +48,7 @@
 std::make_shared(compiler.getTargetOpts(;
 
 const clang::TargetInfo  = compiler.getTarget();
-PtrSize = TInfo.getPointerWidth(0) / 8;
+PtrSize = TInfo.getPointerWidth(clang::LangAS::Default) / 8;
 
 compiler.createFileManager();
 compiler.createSourceManager(compiler.getFileManager());
Index: clang/lib/StaticAnalyzer/Checkers/ObjCContainersASTChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/ObjCContainersASTChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/ObjCContainersASTChecker.cpp
@@ -72,7 +72,7 @@
 public:
   WalkAST(BugReporter , const CheckerBase *checker, AnalysisDeclContext *ac)
   : BR(br), Checker(checker), AC(ac), ASTC(AC->getASTContext()),
-PtrWidth(ASTC.getTargetInfo().getPointerWidth(0)) {}
+PtrWidth(ASTC.getTargetInfo().getPointerWidth(LangAS::Default)) {}
 
   // Statement visitor methods.
   void VisitChildren(Stmt *S);
Index: clang/lib/Sema/SemaType.cpp
===
--- clang/lib/Sema/SemaType.cpp
+++ clang/lib/Sema/SemaType.cpp
@@ -7288,7 +7288,8 @@
 
   // Add address space to type based on its attributes.
   LangAS ASIdx = LangAS::Default;
-  uint64_t PtrWidth = S.Context.getTargetInfo().getPointerWidth(0);
+  uint64_t PtrWidth =
+  S.Context.getTargetInfo().getPointerWidth(LangAS::Default);
   if (PtrWidth == 32) {
 if (Attrs[attr::Ptr64])
   ASIdx = LangAS::ptr64;
Index: clang/lib/Sema/SemaExprCXX.cpp
===
--- clang/lib/Sema/SemaExprCXX.cpp
+++ clang/lib/Sema/SemaExprCXX.cpp
@@ -2661,7 +2661,7 @@
   // tree? Or should the consumer just recalculate the value?
   // FIXME: Using a dummy value will interact poorly with attribute enable_if.
   IntegerLiteral Size(
-  Context, llvm::APInt::getZero(Context.getTargetInfo().getPointerWidth(0)),
+  Context, llvm::APInt::getZero(Context.getTargetInfo().getPointerWidth(LangAS::Default)),
   Context.getSizeType(), SourceLocation());
   AllocArgs.push_back();
 
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -16720,7 +16720,7 @@
   // The type of __null will be int or long, depending on the size of
   // pointers on the target.
   QualType Ty;
-  unsigned pw = Context.getTargetInfo().getPointerWidth(0);
+  unsigned pw = Context.getTargetInfo().getPointerWidth(LangAS::Default);
   if 

[PATCH] D138284: Fix incorrect cast in VisitSYCLUniqueStableNameExpr

[Alexander Richardson via Phabricator via cfe-commits]

arichardson created this revision.
arichardson added reviewers: erichkeane, rjmccall, aaron.ballman.
Herald added subscribers: Naghasan, Anastasia, ebevhan, yaxunl.
Herald added a project: All.
arichardson requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Clang language-level address spaces and LLVM pointer address spaces are
not the same thing (even though they will both have a numeric value of
zero in many cases). LangAS is a enum class to avoid implicit conversions,
but eba69b59d1a30dead07da2c279c8ecfd2b62ba9f 
 avoided 
the compiler error by
adding a static_cast<>. While touching this code, simplify it by using
CreatePointerBitCastOrAddrSpaceCast() which is already a no-op if the types
match.

This changes the code generation for spir64 to place the globals in
addrspace(4). I believe is correct, but it would be good for someone who is
familiar with the target to confirm.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D138284

Files:
  clang/lib/CodeGen/CGExprScalar.cpp
  clang/test/CodeGenSYCL/unique_stable_name.cpp
  clang/test/CodeGenSYCL/unique_stable_name_windows_diff.cpp

Index: clang/test/CodeGenSYCL/unique_stable_name_windows_diff.cpp
===
--- clang/test/CodeGenSYCL/unique_stable_name_windows_diff.cpp
+++ clang/test/CodeGenSYCL/unique_stable_name_windows_diff.cpp
@@ -1,5 +1,5 @@
-// RUN: %clang_cc1 -triple spir64-unknown-unknown-sycldevice -aux-triple x86_64-pc-windows-msvc -fsycl-is-device -disable-llvm-passes -fsycl-is-device -emit-llvm %s -o - | FileCheck %s
-// RUN: %clang_cc1 -triple x86_64-pc-windows-msvc -fsycl-is-device -disable-llvm-passes -fsycl-is-device -emit-llvm %s -o - | FileCheck %s
+// RUN: %clang_cc1 -triple spir64-unknown-unknown-sycldevice -aux-triple x86_64-pc-windows-msvc -fsycl-is-device -disable-llvm-passes -fsycl-is-device -emit-llvm %s -o - | FileCheck %s '-D$ADDRSPACE=addrspace(4) '
+// RUN: %clang_cc1 -triple x86_64-pc-windows-msvc -fsycl-is-device -disable-llvm-passes -fsycl-is-device -emit-llvm %s -o - | FileCheck %s '-D$ADDRSPACE='
 
 
 template
@@ -38,7 +38,7 @@
   // Make sure the following 3 are the same between the host and device compile.
   // Note that these are NOT the same value as eachother, they differ by the
   // signature.
-  // CHECK: private unnamed_addr constant [17 x i8] c"_ZTSZ4mainEUlvE_\00"
-  // CHECK: private unnamed_addr constant [17 x i8] c"_ZTSZ4mainEUliE_\00"
-  // CHECK: private unnamed_addr constant [17 x i8] c"_ZTSZ4mainEUldE_\00"
+  // CHECK: private unnamed_addr [[$ADDRSPACE]]constant [17 x i8] c"_ZTSZ4mainEUlvE_\00"
+  // CHECK: private unnamed_addr [[$ADDRSPACE]]constant [17 x i8] c"_ZTSZ4mainEUliE_\00"
+  // CHECK: private unnamed_addr [[$ADDRSPACE]]constant [17 x i8] c"_ZTSZ4mainEUldE_\00"
 }
Index: clang/test/CodeGenSYCL/unique_stable_name.cpp
===
--- clang/test/CodeGenSYCL/unique_stable_name.cpp
+++ clang/test/CodeGenSYCL/unique_stable_name.cpp
@@ -1,22 +1,22 @@
 // RUN: %clang_cc1 -no-opaque-pointers -triple spir64-unknown-unknown-sycldevice -fsycl-is-device -disable-llvm-passes -emit-llvm %s -o - | FileCheck %s
-// CHECK: @[[LAMBDA_KERNEL3:[^\w]+]] = private unnamed_addr constant [[LAMBDA_K3_SIZE:\[[0-9]+ x i8\]]] c"_ZTSZ4mainEUlPZ4mainEUlvE_E_\00"
-// CHECK: @[[INT1:[^\w]+]] = private unnamed_addr constant [[INT_SIZE:\[[0-9]+ x i8\]]] c"_ZTSi\00"
-// CHECK: @[[STRING:[^\w]+]] = private unnamed_addr constant [[STRING_SIZE:\[[0-9]+ x i8\]]] c"_ZTSAppL_ZZ4mainE1jE_i\00",
-// CHECK: @[[INT2:[^\w]+]] = private unnamed_addr constant [[INT_SIZE]] c"_ZTSi\00"
-// CHECK: @[[LAMBDA_X:[^\w]+]] = private unnamed_addr constant [[LAMBDA_X_SIZE:\[[0-9]+ x i8\]]] c"_ZTSZZ4mainENKUlvE0_clEvEUlvE_\00"
-// CHECK: @[[MACRO_X:[^\w]+]] = private unnamed_addr constant [[MACRO_SIZE:\[[0-9]+ x i8\]]] c"_ZTSZZ4mainENKUlvE0_clEvEUlvE0_\00"
-// CHECK: @[[MACRO_Y:[^\w]+]] =  private unnamed_addr constant [[MACRO_SIZE]] c"_ZTSZZ4mainENKUlvE0_clEvEUlvE1_\00"
-// CHECK: @{{.*}} = private unnamed_addr constant [32 x i8] c"_ZTSZZ4mainENKUlvE0_clEvEUlvE2_\00", align 1
-// CHECK: @{{.*}} = private unnamed_addr constant [32 x i8] c"_ZTSZZ4mainENKUlvE0_clEvEUlvE3_\00", align 1
-// CHECK: @[[MACRO_MACRO_X:[^\w]+]] = private unnamed_addr constant [[MACRO_MACRO_SIZE:\[[0-9]+ x i8\]]] c"_ZTSZZ4mainENKUlvE0_clEvEUlvE4_\00"
-// CHECK: @[[MACRO_MACRO_Y:[^\w]+]] = private unnamed_addr constant [[MACRO_MACRO_SIZE]] c"_ZTSZZ4mainENKUlvE0_clEvEUlvE5_\00"
-// CHECK: @[[INT3:[^\w]+]] = private unnamed_addr constant [[INT_SIZE]] c"_ZTSi\00"
-// CHECK: @[[LAMBDA:[^\w]+]] = private unnamed_addr constant [[LAMBDA_SIZE:\[[0-9]+ x i8\]]] c"_ZTSZZ4mainENKUlvE0_clEvEUlvE_\00"
-// CHECK: @[[LAMBDA_IN_DEP_INT:[^\w]+]] = private unnamed_addr constant [[DEP_INT_SIZE:\[[0-9]+ x i8\]]] 

[PATCH] D111566: [SYCL] Fix function pointer address space

[Alexander Richardson via Phabricator via cfe-commits]

arichardson added inline comments.
Herald added a project: All.



Comment at: clang/lib/AST/ASTContext.cpp:11579
+unsigned ASTContext::getTargetAddressSpace(QualType T) const {
+  return T->isFunctionType() ? getTargetInfo().getProgramAddressSpace()
+ : getTargetAddressSpace(T.getQualifiers());

Can we add a DataLayout aware function to CodeGen instead? That would avoid the 
need for getProgramAddressSpace() in TargetInfo?



Comment at: clang/lib/Basic/TargetInfo.cpp:153
   MaxOpenCLWorkGroupSize = 1024;
+  ProgramAddrSpace = 0;
 }

A bit late to this review (only just noticed it while merging) - but I don't 
like that we end up duplicating even more DataLayout information here - while 
it only affects AVR upstream, downstream CHERI and Morello have to duplicate 
this to Arm/RISC-V/MIPS as well now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111566

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

[PATCH] D135142: Use TI.hasBuiltinAtomic() when setting ATOMIC_*_LOCK_FREE values. NFCI

[Alexander Richardson 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 rG00b1f7a6ab2a: Use TI.hasBuiltinAtomic() when setting 
ATOMIC_*_LOCK_FREE values. NFCI (authored by arichardson).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135142

Files:
  clang/lib/Frontend/InitPreprocessor.cpp


Index: clang/lib/Frontend/InitPreprocessor.cpp
===
--- clang/lib/Frontend/InitPreprocessor.cpp
+++ clang/lib/Frontend/InitPreprocessor.cpp
@@ -299,12 +299,12 @@
 
 /// Get the value the ATOMIC_*_LOCK_FREE macro should have for a type with
 /// the specified properties.
-static const char *getLockFreeValue(unsigned TypeWidth, unsigned InlineWidth) {
+static const char *getLockFreeValue(unsigned TypeWidth, const TargetInfo ) {
   // Fully-aligned, power-of-2 sizes no larger than the inline
   // width will be inlined as lock-free operations.
   // Note: we do not need to check alignment since _Atomic(T) is always
   // appropriately-aligned in clang.
-  if ((TypeWidth & (TypeWidth - 1)) == 0 && TypeWidth <= InlineWidth)
+  if (TI.hasBuiltinAtomic(TypeWidth, TypeWidth))
 return "2"; // "always lock free"
   // We cannot be certain what operations the lib calls might be
   // able to implement as lock-free on future processors.
@@ -1150,11 +1150,9 @@
 
   auto addLockFreeMacros = [&](const llvm::Twine ) {
 // Used by libc++ and libstdc++ to implement ATOMIC__LOCK_FREE.
-unsigned InlineWidthBits = TI.getMaxAtomicInlineWidth();
 #define DEFINE_LOCK_FREE_MACRO(TYPE, Type) 
\
   Builder.defineMacro(Prefix + #TYPE "_LOCK_FREE", 
\
-  getLockFreeValue(TI.get##Type##Width(),  
\
-   InlineWidthBits));
+  getLockFreeValue(TI.get##Type##Width(), TI));
 DEFINE_LOCK_FREE_MACRO(BOOL, Bool);
 DEFINE_LOCK_FREE_MACRO(CHAR, Char);
 if (LangOpts.Char8)
@@ -1167,8 +1165,7 @@
 DEFINE_LOCK_FREE_MACRO(LONG, Long);
 DEFINE_LOCK_FREE_MACRO(LLONG, LongLong);
 Builder.defineMacro(Prefix + "POINTER_LOCK_FREE",
-getLockFreeValue(TI.getPointerWidth(0),
- InlineWidthBits));
+getLockFreeValue(TI.getPointerWidth(0), TI));
 #undef DEFINE_LOCK_FREE_MACRO
   };
   addLockFreeMacros("__CLANG_ATOMIC_");


Index: clang/lib/Frontend/InitPreprocessor.cpp
===
--- clang/lib/Frontend/InitPreprocessor.cpp
+++ clang/lib/Frontend/InitPreprocessor.cpp
@@ -299,12 +299,12 @@
 
 /// Get the value the ATOMIC_*_LOCK_FREE macro should have for a type with
 /// the specified properties.
-static const char *getLockFreeValue(unsigned TypeWidth, unsigned InlineWidth) {
+static const char *getLockFreeValue(unsigned TypeWidth, const TargetInfo ) {
   // Fully-aligned, power-of-2 sizes no larger than the inline
   // width will be inlined as lock-free operations.
   // Note: we do not need to check alignment since _Atomic(T) is always
   // appropriately-aligned in clang.
-  if ((TypeWidth & (TypeWidth - 1)) == 0 && TypeWidth <= InlineWidth)
+  if (TI.hasBuiltinAtomic(TypeWidth, TypeWidth))
 return "2"; // "always lock free"
   // We cannot be certain what operations the lib calls might be
   // able to implement as lock-free on future processors.
@@ -1150,11 +1150,9 @@
 
   auto addLockFreeMacros = [&](const llvm::Twine ) {
 // Used by libc++ and libstdc++ to implement ATOMIC__LOCK_FREE.
-unsigned InlineWidthBits = TI.getMaxAtomicInlineWidth();
 #define DEFINE_LOCK_FREE_MACRO(TYPE, Type) \
   Builder.defineMacro(Prefix + #TYPE "_LOCK_FREE", \
-  getLockFreeValue(TI.get##Type##Width(),  \
-   InlineWidthBits));
+  getLockFreeValue(TI.get##Type##Width(), TI));
 DEFINE_LOCK_FREE_MACRO(BOOL, Bool);
 DEFINE_LOCK_FREE_MACRO(CHAR, Char);
 if (LangOpts.Char8)
@@ -1167,8 +1165,7 @@
 DEFINE_LOCK_FREE_MACRO(LONG, Long);
 DEFINE_LOCK_FREE_MACRO(LLONG, LongLong);
 Builder.defineMacro(Prefix + "POINTER_LOCK_FREE",
-getLockFreeValue(TI.getPointerWidth(0),
- InlineWidthBits));
+getLockFreeValue(TI.getPointerWidth(0), TI));
 #undef DEFINE_LOCK_FREE_MACRO
   };
   addLockFreeMacros("__CLANG_ATOMIC_");
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D116735: [RISCV] Adjust RV64I data layout by using n32:64 in layout string

[Alexander Richardson via Phabricator via cfe-commits]

arichardson added inline comments.



Comment at: llvm/docs/ReleaseNotes.rst:122
   been removed.
+* n32 was added to the RV64I datalayout string.
 

Without additional context I don't think this makes much sense to most readers. 
Before looking at this patch description I would not have been and to say what 
n is used for. 

Maybe something like "i32 has been marked as a legal integer type for RV64, 
improving code generation for some benchmarks"?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116735

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

[PATCH] D135171: FreeBSD: enable __float128 on x86

[Alexander Richardson via Phabricator via cfe-commits]

arichardson accepted this revision.
arichardson added a comment.

Could you add a RUN: line to `clang/test/CodeGenCXX/float128-declarations.cpp? 
Code LGTM.

  // RUN: %clang_cc1 -no-opaque-pointers -emit-llvm -triple 
x86_64-unknown-freebsd -std=c++11 \
  // RUN:   %s -o - | FileCheck %s -check-prefix=CHECK-X86


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135171

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

[PATCH] D135142: Use TI.hasBuiltinAtomic() when setting ATOMIC_*_LOCK_FREE values. NFCI

[Alexander Richardson via Phabricator via cfe-commits]

arichardson created this revision.
arichardson added reviewers: rprichard, efriedma, hfinkel.
Herald added a project: All.
arichardson requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

I noticed that the values for __{CLANG,GCC}_ATOMIC_POINTER_LOCK_FREE were
incorrectly set to 1 instead of two in downstream CHERI targets because
pointers are handled specially there. While fixing this downstream, I
noticed that the existing code could be refactored to use
TargetInfo::hasBuiltinAtomic instead of repeating the almost identical
logic. In theory there could be a difference here since hasBuiltinAtomic() also
returns true for types less than 1 char in size, but since
InitializePredefinedMacros() never passes such a value this change should
not introduce any functional changes.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D135142

Files:
  clang/lib/Frontend/InitPreprocessor.cpp


Index: clang/lib/Frontend/InitPreprocessor.cpp
===
--- clang/lib/Frontend/InitPreprocessor.cpp
+++ clang/lib/Frontend/InitPreprocessor.cpp
@@ -298,12 +298,12 @@
 
 /// Get the value the ATOMIC_*_LOCK_FREE macro should have for a type with
 /// the specified properties.
-static const char *getLockFreeValue(unsigned TypeWidth, unsigned InlineWidth) {
+static const char *getLockFreeValue(unsigned TypeWidth, const TargetInfo ) {
   // Fully-aligned, power-of-2 sizes no larger than the inline
   // width will be inlined as lock-free operations.
   // Note: we do not need to check alignment since _Atomic(T) is always
   // appropriately-aligned in clang.
-  if ((TypeWidth & (TypeWidth - 1)) == 0 && TypeWidth <= InlineWidth)
+  if (TI.hasBuiltinAtomic(TypeWidth, TypeWidth))
 return "2"; // "always lock free"
   // We cannot be certain what operations the lib calls might be
   // able to implement as lock-free on future processors.
@@ -1149,11 +1149,9 @@
 
   auto addLockFreeMacros = [&](const llvm::Twine ) {
 // Used by libc++ and libstdc++ to implement ATOMIC__LOCK_FREE.
-unsigned InlineWidthBits = TI.getMaxAtomicInlineWidth();
 #define DEFINE_LOCK_FREE_MACRO(TYPE, Type) 
\
   Builder.defineMacro(Prefix + #TYPE "_LOCK_FREE", 
\
-  getLockFreeValue(TI.get##Type##Width(),  
\
-   InlineWidthBits));
+  getLockFreeValue(TI.get##Type##Width(), TI));
 DEFINE_LOCK_FREE_MACRO(BOOL, Bool);
 DEFINE_LOCK_FREE_MACRO(CHAR, Char);
 if (LangOpts.Char8)
@@ -1166,8 +1164,7 @@
 DEFINE_LOCK_FREE_MACRO(LONG, Long);
 DEFINE_LOCK_FREE_MACRO(LLONG, LongLong);
 Builder.defineMacro(Prefix + "POINTER_LOCK_FREE",
-getLockFreeValue(TI.getPointerWidth(0),
- InlineWidthBits));
+getLockFreeValue(TI.getPointerWidth(0), TI));
 #undef DEFINE_LOCK_FREE_MACRO
   };
   addLockFreeMacros("__CLANG_ATOMIC_");


Index: clang/lib/Frontend/InitPreprocessor.cpp
===
--- clang/lib/Frontend/InitPreprocessor.cpp
+++ clang/lib/Frontend/InitPreprocessor.cpp
@@ -298,12 +298,12 @@
 
 /// Get the value the ATOMIC_*_LOCK_FREE macro should have for a type with
 /// the specified properties.
-static const char *getLockFreeValue(unsigned TypeWidth, unsigned InlineWidth) {
+static const char *getLockFreeValue(unsigned TypeWidth, const TargetInfo ) {
   // Fully-aligned, power-of-2 sizes no larger than the inline
   // width will be inlined as lock-free operations.
   // Note: we do not need to check alignment since _Atomic(T) is always
   // appropriately-aligned in clang.
-  if ((TypeWidth & (TypeWidth - 1)) == 0 && TypeWidth <= InlineWidth)
+  if (TI.hasBuiltinAtomic(TypeWidth, TypeWidth))
 return "2"; // "always lock free"
   // We cannot be certain what operations the lib calls might be
   // able to implement as lock-free on future processors.
@@ -1149,11 +1149,9 @@
 
   auto addLockFreeMacros = [&](const llvm::Twine ) {
 // Used by libc++ and libstdc++ to implement ATOMIC__LOCK_FREE.
-unsigned InlineWidthBits = TI.getMaxAtomicInlineWidth();
 #define DEFINE_LOCK_FREE_MACRO(TYPE, Type) \
   Builder.defineMacro(Prefix + #TYPE "_LOCK_FREE", \
-  getLockFreeValue(TI.get##Type##Width(),  \
-   InlineWidthBits));
+  getLockFreeValue(TI.get##Type##Width(), TI));
 DEFINE_LOCK_FREE_MACRO(BOOL, Bool);
 DEFINE_LOCK_FREE_MACRO(CHAR, Char);
 if (LangOpts.Char8)
@@ -1166,8 +1164,7 @@
 DEFINE_LOCK_FREE_MACRO(LONG, Long);
 DEFINE_LOCK_FREE_MACRO(LLONG, LongLong);
 Builder.defineMacro(Prefix + "POINTER_LOCK_FREE",
-

[PATCH] D44604: Make stdarg.h compatible with FreeBSD

[Alexander Richardson via Phabricator via cfe-commits]

arichardson abandoned this revision.
arichardson added a comment.
Herald added a subscriber: jrtc27.
Herald added a project: All.

Hopefully no longer required.


Repository:
  rC Clang

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

https://reviews.llvm.org/D44604

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

[PATCH] D134650: [runtimes] Remove all traces of the legacy testing configuration system

[Alexander Richardson via Phabricator via cfe-commits]

arichardson added a comment.

In case anyone else runs into this: It appears this change somehow broke 
incremental builds with `-DLLVM_ENABLE_RUNTIMES=libunwind` (even after deleting 
CMakeCache.txt in the main build dir):

  -- Using libunwind testing configuration: 
/home/alexrichardson/cheri/upstream-llvm-project/libunwind/test/lit.site.cfg.in
  CMake Error: File 
/home/alexrichardson/cheri/upstream-llvm-project/libunwind/test/lit.site.cfg.in 
does not exist.
  CMake Error at 
/home/alexrichardson/cheri/upstream-llvm-project/llvm/cmake/modules/AddLLVM.cmake:1793
 (configure_file):
configure_file Problem configuring file
  Call Stack (most recent call first):

/home/alexrichardson/cheri/upstream-llvm-project/libunwind/test/CMakeLists.txt:45
 (configure_lit_site_cfg)
  
  
  -- Configuring incomplete, errors occurred!

Fortunately this is quite easy to fix: `rm -rf /runtimes` and then 
run `ninja` again.
I'm not sure if this would help (and I don't have a broken build dir anymore), 
but it might be possible to avoid this problem by touching CMakeLists.txt in 
llvm/runtimes?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134650

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

[PATCH] D134337: [clang] [Driver] More flexible rules for loading default configs

[Alexander Richardson via Phabricator via cfe-commits]

arichardson added inline comments.



Comment at: clang/test/Driver/config-file3.c:27
 
-//--- Invocation qqq-clang-g++ tries to find config file qqq-clang-g++.cfg 
first.
+//--- Invocation x86_64-unknown-linux-gnu-clang-g++ tries 
x86_64-unknown-linux-gnu-clang++.cfg first.
 //

sepavloff wrote:
> mgorny wrote:
> > sepavloff wrote:
> > > mgorny wrote:
> > > > sepavloff wrote:
> > > > > mgorny wrote:
> > > > > > sepavloff wrote:
> > > > > > > mgorny wrote:
> > > > > > > > arichardson wrote:
> > > > > > > > > sepavloff wrote:
> > > > > > > > > > Tests must check the case when target prefix is not a real 
> > > > > > > > > > triple as in the original test (qqq-clang).
> > > > > > > > > It would be quite important for me that this continues to 
> > > > > > > > > work. I made use of that in the CheriBSD toolchain when 
> > > > > > > > > creating [[ 
> > > > > > > > > https://github.com/CTSRD-CHERI/cheribuild/blob/master/pycheribuild/projects/cross/llvm.py#L499
> > > > > > > > >  | symlinked binaries to easily build for different ABIs]] 
> > > > > > > > > such as `cheribsd-riscv64-hybrid-clang++` and 
> > > > > > > > > `cheribsd-riscv64-purecap-clang-cpp`. It appears this 
> > > > > > > > > previously only worked if the prefix did not start with a 
> > > > > > > > > valid triple (which is why I put the OS before the 
> > > > > > > > > architecture). I think it would also be nice if the whole 
> > > > > > > > > prefix was checked even if it starts with a valid triple, but 
> > > > > > > > > this does not need to be changed in this patch (haven't 
> > > > > > > > > looked at it in detail so this might actually work).
> > > > > > > > If the prefix is not a valid triple, then clang ignores it and 
> > > > > > > > uses host triple instead. And now we're back to square one. If 
> > > > > > > > I check both variants, it's too complex. If I don't, it's bad 
> > > > > > > > too.
> > > > > > > Our customers use this feature. Target prefix may designate, for 
> > > > > > > example, debug build or build with specific framework.
> > > > > > How are we supposed to avoid the "absurd" case where x86_64 configs 
> > > > > > are loaded for `-m32` invocation then?
> > > > > In this case overloading target does not makes sense. You need to 
> > > > > analyze `RealTriple` in `Driver::loadDefaultConfigFiles` and if it is 
> > > > > wrong, use target prefix as if it is real target.
> > > > What do you mean by "wrong"? The target triple is always a correct 
> > > > triple.
> > > Not triple, sorry. Target prefix taken from `ClangNameParts.TargetPrefix`.
> > Now I'm confused. Are you suggesting that we use prefix instead of target 
> > if it's… incorrect?
> Basically yes.
> 
> We need to support arbitrary target prefixes, because they are used now. If 
> `ClangNameParts.TargetPrefix` is not a valid triple, use it to build config 
> file names.
> 
> We also need to use config files like `x86_64.cfg` where middle components 
> are dropped, they are also used. They also need to support target overloading.
I can replace my approach with simple shell scripts that invoke clang 
--config=..., but IMO it would be nice if the logic was something like the 
first check being
`if "explicit --target empty" and "ClangNameParts.TargetPrefix non-empty" and 
"ClangNameParts.TargetPrefix not a valid triple" -> load 
ClangNameParts.TargetPrefix + ".cfg"` followed by the current logic.
I don't mind either way whether the first check should also load a 
driver-specific config file or not. I'd also be happy if the logic was "try 
loading ClangNameParts.TargetPrefix if explicit --target empty" regardless of 
whether it is a valid triple or not.
Does this approach sound reasonable?


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

https://reviews.llvm.org/D134337

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

[PATCH] D134337: [clang] [Driver] More flexible rules for loading default configs

[Alexander Richardson via Phabricator via cfe-commits]

arichardson added inline comments.



Comment at: clang/test/Driver/config-file3.c:27
 
-//--- Invocation qqq-clang-g++ tries to find config file qqq-clang-g++.cfg 
first.
+//--- Invocation x86_64-unknown-linux-gnu-clang-g++ tries 
x86_64-unknown-linux-gnu-clang++.cfg first.
 //

sepavloff wrote:
> Tests must check the case when target prefix is not a real triple as in the 
> original test (qqq-clang).
It would be quite important for me that this continues to work. I made use of 
that in the CheriBSD toolchain when creating [[ 
https://github.com/CTSRD-CHERI/cheribuild/blob/master/pycheribuild/projects/cross/llvm.py#L499
 | symlinked binaries to easily build for different ABIs]] such as 
`cheribsd-riscv64-hybrid-clang++` and `cheribsd-riscv64-purecap-clang-cpp`. It 
appears this previously only worked if the prefix did not start with a valid 
triple (which is why I put the OS before the architecture). I think it would 
also be nice if the whole prefix was checked even if it starts with a valid 
triple, but this does not need to be changed in this patch (haven't looked at 
it in detail so this might actually work).


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

https://reviews.llvm.org/D134337

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

[PATCH] D134671: [Driver] Prevent Mips specific code from claiming -mabi argument on other targets.

[Alexander Richardson via Phabricator via cfe-commits]

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

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134671

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

[PATCH] D134671: [Driver] Prevent Mips specific code from claiming -mabi argument on other targets.

[Alexander Richardson via Phabricator via cfe-commits]

arichardson added inline comments.



Comment at: clang/lib/Driver/Driver.cpp:637
   // accordingly to provided ABI name.
-  A = Args.getLastArg(options::OPT_mabi_EQ);
+  A = Args.getLastArgNoClaim(options::OPT_mabi_EQ);
   if (A && Target.isMIPS()) {

Would it make more sense to move this into the if? It makes the diff bigger 
since everything inside has to be reindented, but only querying the flag if 
target is MIPS seems cleaner to me than the noclaim+a->claim approach.



Comment at: clang/lib/Driver/Driver.cpp:661
   // provided architecture name
   A = Args.getLastArg(options::OPT_march_EQ);
   if (A && Target.isRISCV()) {

Don't we have the same problem here?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134671

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

[PATCH] D108212: Emit offsetof values as notes when a static_assert fails

[Alexander Richardson via Phabricator via cfe-commits]

arichardson abandoned this revision.
arichardson added a comment.
Herald added a project: All.

No longer needed


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108212

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

[PATCH] D108211: Emit sizeof/alignof values as notes when a static_assert fails

[Alexander Richardson via Phabricator via cfe-commits]

arichardson abandoned this revision.
arichardson added a comment.

Would require significant work to still be useful now that we print the value 
of expressions (e.g. only print for more complex expressions).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108211

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

[PATCH] D132608: [CMake] Clean up CMake binary dir handling

[Alexander Richardson via Phabricator via cfe-commits]

arichardson added inline comments.



Comment at: cmake/Modules/GNUBinaryDirs.cmake:6
+if (NOT DEFINED CMAKE_BINARY_BINDIR)
+  set(CMAKE_BINARY_BINDIR "${CMAKE_BINARY_BINDIR}/bin")
+endif()

I find this name a bit confusing, maybe something like `CMAKE_BUILD_BINDIR` 
(and the same for _LIBDIR/_INCLUDEDIR would be less surprising? That way it's 
clear that we are referring to binaries/libraries/includes in the build output 
(and it mirrors CMAKE_INSTALL_INCLUDEDIR, etc.) 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132608

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

[PATCH] D108211: Emit sizeof/alignof values as notes when a static_assert fails

[Alexander Richardson via Phabricator via cfe-commits]

arichardson added a reviewer: tbaeder.
arichardson added a comment.

This is less useful now that 09117b21890c652994f7ada0229d309b35b44259 
 / D130894 
 has landed, but it might still be worth 
including (e.g. for expressions that include multiple sizeof() values?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108211

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

[PATCH] D108211: Emit sizeof/alignof values as notes when a static_assert fails

[Alexander Richardson via Phabricator via cfe-commits]

arichardson updated this revision to Diff 455258.
arichardson marked 2 inline comments as done.
arichardson added a comment.
Herald added a project: All.

rebase and address feedback


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108211

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/lib/Sema/SemaTemplate.cpp
  clang/test/CXX/dcl.decl/dcl.meaning/dcl.array/p3.cpp
  clang/test/Parser/objc-static-assert.mm
  clang/test/Sema/static-assert.c
  clang/test/SemaCXX/constant-expression-cxx11.cpp
  clang/test/SemaCXX/static-assert-cxx17.cpp
  clang/test/SemaCXX/static-assert.cpp
  clang/test/SemaTemplate/overload-candidates.cpp

Index: clang/test/SemaTemplate/overload-candidates.cpp
===
--- clang/test/SemaTemplate/overload-candidates.cpp
+++ clang/test/SemaTemplate/overload-candidates.cpp
@@ -62,6 +62,8 @@
 
 template struct NonTemplateFunction {
   typename boost::enable_if::type f(); // expected-error{{failed requirement 'sizeof(char) == 4'; 'enable_if' cannot be used to disable this declaration}}
+  // expected-note@-1{{expression evaluates to '1 == 4'}}
+  // expected-note@-2{{with 'sizeof(char)' equal to 1}}
 };
 NonTemplateFunction NTFC; // expected-note{{here}}
 
Index: clang/test/SemaCXX/static-assert.cpp
===
--- clang/test/SemaCXX/static-assert.cpp
+++ clang/test/SemaCXX/static-assert.cpp
@@ -23,7 +23,9 @@
 
 template struct S {
 static_assert(sizeof(T) > sizeof(char), "Type not big enough!"); // expected-error {{static assertion failed due to requirement 'sizeof(char) > sizeof(char)': Type not big enough!}} \
- // expected-note {{1 > 1}}
+ // expected-note {{1 > 1}} \
+ // expected-note {{with 'sizeof(char)' equal to 1}} \
+ // expected-note {{with 'sizeof(char)' equal to 1}}
 };
 
 S s1; // expected-note {{in instantiation of template class 'S' requested here}}
@@ -264,3 +266,30 @@
 
 
 }
+
+struct IntAndPointer {
+  int i;
+  void *p;
+};
+static_assert(sizeof(IntAndPointer) == 4, "message");
+// expected-error@-1{{static assertion failed}} \
+// expected-note@-1{{expression evaluates to '16 == 4'}} \
+// expected-note@-1{{with 'sizeof(IntAndPointer)' equal to 16}}
+static_assert(alignof(IntAndPointer) == 4, "message");
+// expected-error@-1{{static assertion failed}} \
+// expected-note@-1{{expression evaluates to '8 == 4'}} \
+// expected-note@-1{{with 'alignof(IntAndPointer)' equal to 8}}
+static_assert(alignof(IntAndPointer) == sizeof(IntAndPointer), "message");
+// expected-error@-1{{static assertion failed}} \
+// expected-note@-1{{expression evaluates to '8 == 16'}} \
+// expected-note@-1{{with 'alignof(IntAndPointer)' equal to 8}} \
+// expected-note@-1{{with 'sizeof(IntAndPointer)' equal to 16}}
+static_assert(alignof(IntAndPointer) + sizeof(int) == sizeof(IntAndPointer), "message");
+// expected-error@-1{{static assertion failed}} \
+// expected-note@-1{{expression evaluates to '12 == 16'}} \
+// expected-note@-1{{with 'alignof(IntAndPointer)' equal to 8}} \
+// expected-note@-1{{with 'sizeof(int)' equal to 4}} \
+// expected-note@-1{{with 'sizeof(IntAndPointer)' equal to 16}}
+/// Should not print the sizeof(int) value here since it's not evaluated.
+static_assert(std::is_same::value, "message");
+// expected-error@-1{{static assertion failed}}
Index: clang/test/SemaCXX/static-assert-cxx17.cpp
===
--- clang/test/SemaCXX/static-assert-cxx17.cpp
+++ clang/test/SemaCXX/static-assert-cxx17.cpp
@@ -89,7 +89,8 @@
   // expected-error@-1{{static assertion failed due to requirement 'int(0)'}}
   static_assert(sizeof(X) == 0);
   // expected-error@-1{{static assertion failed due to requirement 'sizeof(X) == 0'}} \
-  // expected-note@-1 {{evaluates to '8 == 0'}}
+  // expected-note@-1 {{evaluates to '8 == 0'}} \
+  // expected-note@-1 {{with 'sizeof(X)' equal to 8}}
   static_assert((const X *)nullptr);
   // expected-error@-1{{static assertion failed due to requirement '(const X *)nullptr'}}
   static_assert(static_cast *>(nullptr));
@@ -98,7 +99,8 @@
   // expected-error@-1{{static assertion failed due to requirement '(const X[0]){} == nullptr'}}
   static_assert(sizeof(X().X::~X())>) == 0);
   // expected-error@-1{{static assertion failed due to requirement 'sizeof(X) == 0'}} \
-  // expected-note@-1 {{evaluates to '8 == 0'}}
+  // expected-note@-1 {{evaluates to '8 == 0'}} \
+  // expected-note@-1 {{with 'sizeof(X)' equal to 8}}
   

[PATCH] D131155: [clang] Expand array expressions if the filler expression's filler is element dependent

[Alexander Richardson via Phabricator via cfe-commits]

arichardson added inline comments.



Comment at: clang/test/SemaCXX/constexpr-array-init.cpp:3
+
+
+/// expected-no-diagnostics

Nit: It might be helpful to add a comment to this test explaining what it's 
testing. This makes it easier to diagnose tests failures (e.g. in downstream 
forks) without having to rely on git blame.


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

https://reviews.llvm.org/D131155

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

[PATCH] D130089: update-test-checks: safely handle tests with #if's

[Alexander Richardson via Phabricator via cfe-commits]

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



Comment at: 
llvm/test/tools/UpdateTestChecks/update_llc_test_checks/Inputs/arm_function_name.ll:2
 ; Check that we accept functions with '$' in the name.
 ; TODO: This is not handled correcly on 32bit ARM and needs to be fixed.
 

Remove this TODO?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130089

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

[PATCH] D128625: [RISCV][Driver] Fix baremetal `GCCInstallation` paths

[Alexander Richardson via Phabricator via cfe-commits]

arichardson added inline comments.



Comment at: clang/lib/Driver/ToolChains/RISCVToolchain.cpp:54
+
+  // Set alias for "riscv{64|32}-unknown-unknown-elf"
+  SmallVector TripleAliases;

anton-afanasyev wrote:
> arichardson wrote:
> > This seems like the wrong place to add this workaround, shouldn't the 
> > change be in `GCCInstallation::init`? That way targets other than RISC-V 
> > also benefit from this fix.
> > 
> `GCCInstallation` knows nothing about triple equivalence of the specific 
> targets, but provides `TripleAliases` init variable for installation target 
> callers. Other targets are to do the same initialization as we do here in 
> case of different normalized and layout-used triple names.
What I was trying to say is that the normalization adding `-unknown` affects 
all targets, so maybe `GCCInstallation::init()` should add aliases that have 
the `-unknown` components removed rather than doing this at the call site. This 
way the alias computation does not need to be moved to each target info.

However, I also think we should be passing through the triple as passed to 
--target= rather than the normalized triple.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128625

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

[PATCH] D128625: [RISCV][Driver] Fix baremetal `GCCInstallation` paths

[Alexander Richardson via Phabricator via cfe-commits]

arichardson added inline comments.



Comment at: clang/lib/Driver/ToolChains/RISCVToolchain.cpp:54
+
+  // Set alias for "riscv{64|32}-unknown-unknown-elf"
+  SmallVector TripleAliases;

This seems like the wrong place to add this workaround, shouldn't the change be 
in `GCCInstallation::init`? That way targets other than RISC-V also benefit 
from this fix.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128625

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

[PATCH] D122335: [clang] Emit crash reproduction as a single tar file

[Alexander Richardson via Phabricator via cfe-commits]

arichardson added inline comments.



Comment at: clang/CMakeLists.txt:520
+   ON "${CMAKE_BUILD_TYPE} MATCHES Debug" OFF)
+if(CLANG_CRASH_SAVE_TEMPS)
+  add_definitions(-DCLANG_CRASH_SAVE_TEMPS)

Could you add this to the config.h header instead?



Comment at: clang/lib/Driver/Driver.cpp:1434
 StringRef AdditionalInformation, CompilationDiagnosticReport *Report) {
+#ifdef CLANG_CRASH_SAVE_TEMPS
+  constexpr bool SaveReproTemps = true;

This could be simplified with `#cmakedefine01` in the config header



Comment at: clang/test/Driver/crash-report-clang-cl.cpp:1
+// UNSUPPORTED: windows
+

I think it would be cleaner to add a new feature if tar is available in $PATH 
instead of ignoring all windows bots. Some might have tar installed.



Comment at: clang/test/Driver/crash-report-clang-cl.cpp:8
 // RUN: -fcrash-diagnostics-dir=%t -- %s 2>&1 | FileCheck %s
-// RUN: cat %t/crash-report-clang-cl-*.cpp | FileCheck --check-prefix=CHECKSRC 
%s
-// RUN: cat %t/crash-report-clang-cl-*.sh | FileCheck --check-prefix=CHECKSH %s
+// RUN: tar xOf %t/*.tar --wildcards "*/tmp/crash-report-clang-cl-*.cpp" \
+// RUN: | FileCheck --check-prefix=CHECKSRC %s

Alternatively, if we had an env var/command line option to keep the cpp/sh, we 
could use that to avoid the dependency on tar in this test.



Comment at: clang/test/Driver/crash-report-save-temps.c:14
+
+// RUN: false

Is this line intentional?


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

https://reviews.llvm.org/D122335

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

[PATCH] D122335: [clang] Emit crash reproduction as a single tar file

[Alexander Richardson via Phabricator via cfe-commits]

arichardson added a comment.

In D122335#3404358 , @jrtc27 wrote:

> As a developer who often deals with crashes locally this is more annoying; 
> currently I can just point tools at the shell script and C file in /tmp and 
> let them go to work reducing, but now I have to also extract the files

+1.

I can see this being helpful for end-users who now just have to provide 
developers with a single file, but it does make things more awkward for me as a 
developer. How about an environment variable and/or a CMake option to skip 
archiving that can be used locally.


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

https://reviews.llvm.org/D122335

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

[PATCH] D109611: Fix CLANG_ENABLE_STATIC_ANALYZER=OFF building all analyzer source

[Alexander Richardson via Phabricator via cfe-commits]

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

Have to fix `cmake -GNinja -DCLANG_ENABLE_STATIC_ANALYZER=OFF 
-DLLVM_ENABLE_PROJECTS="llvm;clang;clang-tools-extra" -DCLANG_ENABLE_ARCMT=OFF 
../llvm`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109611

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

[PATCH] D109611: Fix CLANG_ENABLE_STATIC_ANALYZER=OFF building all analyzer source

[Alexander Richardson via Phabricator via cfe-commits]

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

I will try to get back to this soon, but it will probably not be this week.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109611

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

[PATCH] D100810: [llvm] Use `GNUInstallDirs` to support custom installation dirs

[Alexander Richardson via Phabricator via cfe-commits]

arichardson added inline comments.



Comment at: llvm/CMakeLists.txt:5
 
+include(GNUInstallDirs)
+

arichardson wrote:
> This seems to be causing the following warning for me:
> 
> ```
> CMake Warning (dev) at 
> /opt/clion-2021.2/bin/cmake/linux/share/cmake-3.20/Modules/GNUInstallDirs.cmake:236
>  (message):
>   Unable to determine default CMAKE_INSTALL_LIBDIR directory because no
>   target architecture is known.  Please enable at least one language before
>   including GNUInstallDirs.
> ```
Moving it below `project(LLVM` should fix that, but I'm not sure if there is a 
reason that it's up here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100810

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

[PATCH] D100810: [llvm] Use `GNUInstallDirs` to support custom installation dirs

[Alexander Richardson via Phabricator via cfe-commits]

arichardson added inline comments.



Comment at: llvm/CMakeLists.txt:5
 
+include(GNUInstallDirs)
+

This seems to be causing the following warning for me:

```
CMake Warning (dev) at 
/opt/clion-2021.2/bin/cmake/linux/share/cmake-3.20/Modules/GNUInstallDirs.cmake:236
 (message):
  Unable to determine default CMAKE_INSTALL_LIBDIR directory because no
  target architecture is known.  Please enable at least one language before
  including GNUInstallDirs.
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100810

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

[PATCH] D71026: Fix LLVM_ENABLE_MODULES=ON + BUILD_SHARED_LIBS=ON build

[Alexander Richardson via Phabricator via cfe-commits]

arichardson abandoned this revision.
arichardson added a comment.

I believe this is no longer necessary.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71026

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

[PATCH] D102669: [analyzer][ctu] Fix wrong 'multiple definitions' errors caused by space characters in lookup names when parsing the ctu index file

[Alexander Richardson via Phabricator via cfe-commits]

arichardson added inline comments.



Comment at: clang/test/Analysis/ctu-lookup-name-with-space.cpp:13
+// RUN:   -verify %s
+
+void importee();

OikawaKirie wrote:
> Adding this line here.
Disabling the test on non- Linux is not a good idea IMO since it means we lose 
coverage on other platforms. My guess is that you just need to specify an 
explicit triple in the clang invocations.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102669

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

[PATCH] D113372: [Driver] Add CLANG_DEFAULT_PIE_ON_LINUX to emulate GCC --enable-default-pie

[Alexander Richardson via Phabricator via cfe-commits]

arichardson added inline comments.



Comment at: clang/CMakeLists.txt:232
+if(CLANG_DEFAULT_PIE_ON_LINUX)
+  set(CLANG_DEFAULT_PIE_ON_LINUX 1)
+endif()

Can these 3 lines be removed after D115751?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113372

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

[PATCH] D110257: [CFE][Codegen] Make sure to maintain the contiguity of all the static allocas

[Alexander Richardson via Phabricator via cfe-commits]

arichardson added inline comments.



Comment at: clang/lib/CodeGen/CGExpr.cpp:115
+if (AllocaInsertedAtAllocaInsertPt)
+  AllocaAddrSpaceInsertPt = dyn_cast(V)->getIterator();
   }

Shouldn't this use `cast` instead?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110257

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

[PATCH] D110612: [Utils] Use common substs in update_cc_test_checks

[Alexander Richardson via Phabricator via cfe-commits]

arichardson added reviewers: jdoerfert, ggeorgakoudis.
arichardson added inline comments.



Comment at: llvm/utils/update_cc_test_checks.py:223
 def exec_run_line(exe):
   popen = subprocess.Popen(exe, stdout=subprocess.PIPE, 
stderr=subprocess.PIPE, universal_newlines=True)
   stdout, stderr = popen.communicate()

I think `common.applySubstitutions` should be called here and not below.



Comment at: llvm/utils/update_cc_test_checks.py:242
+subs = common.getSubstitutions(ti.path)
+subs.append(('%t', tempfile.NamedTemporaryFile().name))
 

Shouldn't %t be supported in all tools? We could move this common.py. However, 
before doing that we should also fix temporary files to be cleaned up on exit. 
Looks like that bug was introduced in D98712.

I think the easiest solution would be to use a filename inside a 
TemporaryDirectory() that is cleaned up on exit.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110612

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

[PATCH] D109611: Fix CLANG_ENABLE_STATIC_ANALYZER=OFF building all analyzer source

[Alexander Richardson via Phabricator via cfe-commits]

arichardson added a comment.

In D109611#3010353 , @thakis wrote:

> This breaks the build for us:
>
>   Running cmake -GNinja -DCMAKE_BUILD_TYPE=Release 
> -DLLVM_ENABLE_ASSERTIONS=ON 
> '-DLLVM_ENABLE_PROJECTS=clang;compiler-rt;lld;chrometools;clang-tools-extra' 
> -DLLVM_CHECK_ENABLED_PROJECTS=OFF 
> '-DLLVM_TARGETS_TO_BUILD=AArch64;ARM;Mips;PowerPC;SystemZ;WebAssembly;X86' 
> -DLLVM_ENABLE_PIC=OFF -DLLVM_ENABLE_UNWIND_TABLES=OFF 
> -DLLVM_ENABLE_TERMINFO=OFF -DLLVM_ENABLE_Z3_SOLVER=OFF 
> -DCLANG_PLUGIN_SUPPORT=OFF -DCLANG_ENABLE_STATIC_ANALYZER=OFF 
> -DCLANG_ENABLE_ARCMT=OFF '-DBUG_REPORT_URL=https://crbug.com and run 
> tools/clang/scripts/process_crashreports.py (only works inside Google) which 
> will upload a report' -DLLVM_INCLUDE_GO_TESTS=OFF 
> -DENABLE_X86_RELAX_RELOCATIONS=NO -DLLVM_ENABLE_DIA_SDK=OFF 
> '-DCOMPILER_RT_SANITIZERS_TO_BUILD=asan;dfsan;msan;hwasan;tsan;cfi' 
> -DLLVM_ENABLE_PER_TARGET_RUNTIME_DIR=OFF 
> -DLLVM_LOCAL_RPATH=/b/s/w/ir/cache/builder/src/third_party/llvm-build-tools/gcc-10.2.0-trusty/lib64
>  
> '-DCOMPILER_RT_TEST_COMPILER_CFLAGS=--gcc-toolchain=/b/s/w/ir/cache/builder/src/third_party/llvm-build-tools/gcc-10.2.0-trusty
>  
> -Wl,-rpath,/b/s/w/ir/cache/builder/src/third_party/llvm-build-tools/gcc-10.2.0-trusty/lib64
>  
> -Wl,-rpath,/b/s/w/ir/cache/builder/src/third_party/llvm-build-tools/gcc-10.2.0-trusty/lib32'
>  -DLLVM_ENABLE_LIBXML2=FORCE_ON 
> -DCMAKE_C_COMPILER=/b/s/w/ir/cache/builder/src/third_party/llvm-build-tools/gcc-10.2.0-trusty/bin/gcc
>  
> -DCMAKE_CXX_COMPILER=/b/s/w/ir/cache/builder/src/third_party/llvm-build-tools/gcc-10.2.0-trusty/bin/g++
>  -DCOMPILER_RT_BUILD_CRT=OFF -DCOMPILER_RT_BUILD_LIBFUZZER=OFF 
> -DCOMPILER_RT_BUILD_MEMPROF=OFF -DCOMPILER_RT_BUILD_ORC=OFF 
> -DCOMPILER_RT_BUILD_PROFILE=ON -DCOMPILER_RT_BUILD_SANITIZERS=ON 
> -DCOMPILER_RT_BUILD_XRAY=OFF -DCOMPILER_RT_BUILD_BUILTINS=OFF 
> -DCMAKE_C_FLAGS=-DLLVM_FORCE_HEAD_REVISION 
> -DCMAKE_CXX_FLAGS=-DLLVM_FORCE_HEAD_REVISION -DCMAKE_EXE_LINKER_FLAGS= 
> -DCMAKE_SHARED_LINKER_FLAGS= -DCMAKE_MODULE_LINKER_FLAGS= 
> -DCMAKE_INSTALL_PREFIX=/b/s/w/ir/cache/builder/src/third_party/llvm-build/Release+Asserts
>  -DCHROMIUM_TOOLS_SRC=/b/s/w/ir/cache/builder/src/tools/clang 
> '-DCHROMIUM_TOOLS=translation_unit;blink_gc_plugin;plugins' 
> -DLLVM_DEFAULT_TARGET_TRIPLE=x86_64-unknown-linux-gnu 
> /b/s/w/ir/cache/builder/src/third_party/llvm/llvm
>   
>   
>   -- Configuring done
>   CMake Error: install(EXPORT "ClangTargets" ...) includes target "clangTidy" 
> which requires target "clangStaticAnalyzerCore" that is not in any export set.
>   CMake Error: install(EXPORT "ClangTargets" ...) includes target "clangTidy" 
> which requires target "clangStaticAnalyzerFrontend" that is not in any export 
> set.
>   CMake Error: install(EXPORT "ClangTargets" ...) includes target 
> "clangTidyMPIModule" which requires target "clangStaticAnalyzerCheckers" that 
> is not in any export set.
>   CMake Error in 
> /b/s/w/ir/cache/builder/src/third_party/llvm/clang/cmake/modules/CMakeLists.txt:
> export called with target "clangTidy" which requires target
> "clangStaticAnalyzerCore" that is not in any export set.
>   
>   
>   CMake Error in 
> /b/s/w/ir/cache/builder/src/third_party/llvm/clang/cmake/modules/CMakeLists.txt:
> export called with target "clangTidy" which requires target
> "clangStaticAnalyzerFrontend" that is not in any export set.
>   
>   
>   CMake Error in 
> /b/s/w/ir/cache/builder/src/third_party/llvm/clang/cmake/modules/CMakeLists.txt:
> export called with target "clangTidyMPIModule" which requires target
> "clangStaticAnalyzerCheckers" that is not in any export set.
>
> Are we holding it wrong?

Hmm that sounds like it might be awkward to fix. I'll try to look into it ASAP 
(tomorrow morning UK time). In the mean time feel free to revert if this is a 
blocker for you.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109611

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

[PATCH] D109611: Fix CLANG_ENABLE_STATIC_ANALYZER=OFF building all analyzer source

[Alexander Richardson via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG6d7b3d6b3a8d: Fix CLANG_ENABLE_STATIC_ANALYZER=OFF building 
all analyzer source (authored by arichardson).

Changed prior to commit:
  https://reviews.llvm.org/D109611?vs=371944=373554#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109611

Files:
  clang/cmake/modules/AddClang.cmake
  clang/lib/StaticAnalyzer/CMakeLists.txt
  llvm/cmake/modules/AddLLVM.cmake


Index: llvm/cmake/modules/AddLLVM.cmake
===
--- llvm/cmake/modules/AddLLVM.cmake
+++ llvm/cmake/modules/AddLLVM.cmake
@@ -429,10 +429,13 @@
 #  This is used to specify that this is a component library of
 #  LLVM which means that the source resides in llvm/lib/ and it is a
 #  candidate for inclusion into libLLVM.so.
+#   EXCLUDE_FROM_ALL
+#  Do not build this library as part of the default target, only
+#  if explicitly requested or when linked against.
 #   )
 function(llvm_add_library name)
   cmake_parse_arguments(ARG
-
"MODULE;SHARED;STATIC;OBJECT;DISABLE_LLVM_LINK_LLVM_DYLIB;SONAME;NO_INSTALL_RPATH;COMPONENT_LIB"
+
"MODULE;SHARED;STATIC;OBJECT;DISABLE_LLVM_LINK_LLVM_DYLIB;SONAME;NO_INSTALL_RPATH;COMPONENT_LIB;EXCLUDE_FROM_ALL"
 "OUTPUT_NAME;PLUGIN_TOOL;ENTITLEMENTS;BUNDLE_PATH"
 "ADDITIONAL_HEADERS;DEPENDS;LINK_COMPONENTS;LINK_LIBS;OBJLIBS"
 ${ARGN})
@@ -535,6 +538,9 @@
 
 # FIXME: Add name_static to anywhere in TARGET ${name}'s PROPERTY.
 set(ARG_STATIC)
+if(ARG_EXCLUDE_FROM_ALL OR EXCLUDE_FROM_ALL)
+  set_target_properties(${name_static} PROPERTIES EXCLUDE_FROM_ALL ON)
+endif()
   endif()
 
   if(ARG_MODULE)
@@ -546,6 +552,10 @@
 add_library(${name} STATIC ${ALL_FILES})
   endif()
 
+  if(ARG_EXCLUDE_FROM_ALL OR EXCLUDE_FROM_ALL)
+set_target_properties(${name} PROPERTIES EXCLUDE_FROM_ALL ON)
+  endif()
+
   if(ARG_COMPONENT_LIB)
 set_target_properties(${name} PROPERTIES LLVM_COMPONENT TRUE)
 set_property(GLOBAL APPEND PROPERTY LLVM_COMPONENT_LIBS ${name})
Index: clang/lib/StaticAnalyzer/CMakeLists.txt
===
--- clang/lib/StaticAnalyzer/CMakeLists.txt
+++ clang/lib/StaticAnalyzer/CMakeLists.txt
@@ -1,3 +1,10 @@
+# These directories can significantly impact build time, only build
+# them if anything depends on the clangStaticAnalyzer* libraries.
+if(NOT CLANG_ENABLE_STATIC_ANALYZER)
+  set_property(DIRECTORY PROPERTY EXCLUDE_FROM_ALL ON)
+  set(EXCLUDE_FROM_ALL ON)
+endif()
+
 add_subdirectory(Core)
 add_subdirectory(Checkers)
 add_subdirectory(Frontend)
Index: clang/cmake/modules/AddClang.cmake
===
--- clang/cmake/modules/AddClang.cmake
+++ clang/cmake/modules/AddClang.cmake
@@ -100,7 +100,12 @@
   # The Xcode generator doesn't handle object libraries correctly.
   list(APPEND LIBTYPE OBJECT)
 endif()
-set_property(GLOBAL APPEND PROPERTY CLANG_STATIC_LIBS ${name})
+if (NOT EXCLUDE_FROM_ALL)
+  # Only include libraries that don't have EXCLUDE_FROM_ALL set. This
+  # ensure that the clang static analyzer libraries are not compiled
+  # as part of clang-shlib if CLANG_ENABLE_STATIC_ANALYZER=OFF.
+  set_property(GLOBAL APPEND PROPERTY CLANG_STATIC_LIBS ${name})
+endif()
   endif()
   llvm_add_library(${name} ${LIBTYPE} ${ARG_UNPARSED_ARGUMENTS} ${srcs})
 
@@ -110,8 +115,11 @@
   endif()
 
   foreach(lib ${libs})
-if(TARGET ${lib})
+   if(TARGET ${lib})
   target_link_libraries(${lib} INTERFACE ${LLVM_COMMON_LIBS})
+  if (EXCLUDE_FROM_ALL)
+continue()
+  endif()
 
   if (NOT LLVM_INSTALL_TOOLCHAIN_ONLY OR ARG_INSTALL_WITH_TOOLCHAIN)
 get_target_export_arg(${name} Clang export_to_clangtargets UMBRELLA 
clang-libraries)


Index: llvm/cmake/modules/AddLLVM.cmake
===
--- llvm/cmake/modules/AddLLVM.cmake
+++ llvm/cmake/modules/AddLLVM.cmake
@@ -429,10 +429,13 @@
 #  This is used to specify that this is a component library of
 #  LLVM which means that the source resides in llvm/lib/ and it is a
 #  candidate for inclusion into libLLVM.so.
+#   EXCLUDE_FROM_ALL
+#  Do not build this library as part of the default target, only
+#  if explicitly requested or when linked against.
 #   )
 function(llvm_add_library name)
   cmake_parse_arguments(ARG
-"MODULE;SHARED;STATIC;OBJECT;DISABLE_LLVM_LINK_LLVM_DYLIB;SONAME;NO_INSTALL_RPATH;COMPONENT_LIB"
+"MODULE;SHARED;STATIC;OBJECT;DISABLE_LLVM_LINK_LLVM_DYLIB;SONAME;NO_INSTALL_RPATH;COMPONENT_LIB;EXCLUDE_FROM_ALL"
 "OUTPUT_NAME;PLUGIN_TOOL;ENTITLEMENTS;BUNDLE_PATH"
 "ADDITIONAL_HEADERS;DEPENDS;LINK_COMPONENTS;LINK_LIBS;OBJLIBS"
 ${ARGN})
@@ -535,6 +538,9 @@
 
 # FIXME: Add name_static to 

[PATCH] D109841: Fix vtbl field addr space

[Alexander Richardson via Phabricator via cfe-commits]

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

Thanks, I can confirm that this fixes the assertions we were seeing after 
merging D103835 . A few minor suggestions 
below:

There's a typo in the commit message, I'd maybe change it to:
`Storing the vtable field of an object should use the same address space as the 
this pointer.`
And maybe change `This caused issue for some out of tree project.` to something 
like `This assumption (added in 054cc3b1b469de4b0cb25d1dc3af43c679c5dc44) 
caused issues for the out-of-tree CHERI targets`.




Comment at: clang/lib/CodeGen/CGClass.cpp:2522-2523
+  // vtable field is is derived from `this` pointer, therefore they should be 
in
+  // the same addr space. Note it may not be the default address space of LLVM
+  // IR.
   VTableField = Builder.CreatePointerBitCastOrAddrSpaceCast(

Maybe this is clearer?



Comment at: clang/lib/CodeGen/CGClass.cpp:2526-2527
+  VTableField, VTablePtrTy->getPointerTo(ThisAddrSpace));
   VTableAddressPoint = Builder.CreatePointerBitCastOrAddrSpaceCast(
   VTableAddressPoint, VTablePtrTy);
 

As noted by @rjmccall, changing this line still allows all tests to pass 
(including our newly added out-of-tree CHERI ones).


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

https://reviews.llvm.org/D109841

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

[PATCH] D103835: [CUDA][HIP] Fix store of vtbl in ctor

[Alexander Richardson via Phabricator via cfe-commits]

arichardson added a comment.

In D103835#2999731 , @yaxunl wrote:

> In D103835#2999545 , @arichardson 
> wrote:
>
>> Merging this change broke our out-of-tree CHERI targets (and Arm Morello). 
>> We use addrspace(200) for *all* globals (including vtables). It would have 
>> been nice if I had been added to this review considering that I added line 
>> you are changing here.
>>
>> If vtables are not in the default globals address space, I think we need 
>> another way of expressing this. I think 
>> `CGM.getContext().getTargetAddressSpace(LangAS::Default))` should also be 
>> correct for your use-case?
>
> vtbl addr space should be the same as `this` pointer. If I use addr space of 
> `this` pointer and not assuming it is default addr space, will it work for 
> you? Thanks.

Yes that would also work. Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103835

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

[PATCH] D103835: [CUDA][HIP] Fix store of vtbl in ctor

[Alexander Richardson via Phabricator via cfe-commits]

arichardson added a comment.

Merging this change broke our out-of-tree CHERI targets (and Arm Morello). We 
use addrspace(200) for *all* globals (including vtables). It would have been 
nice if I had been added to this review considering that I added line you are 
changing here.

If vtables are not in the default globals address space, I think we need 
another way of expressing this. I think 
`CGM.getContext().getTargetAddressSpace(LangAS::Default))` should also be 
correct for your use-case?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103835

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

[PATCH] D109611: Fix CLANG_ENABLE_STATIC_ANALYZER=OFF building all analyzer source

[Alexander Richardson via Phabricator via cfe-commits]

arichardson added a comment.

In D109611#2997236 , @thakis wrote:

> Can you just set `CLANG_TIDY_ENABLE_STATIC_ANALYZER=OFF` too if you care 
> about this?

That doesn't have any effect on the libraries linked into clang-shlib. I am not 
building clang-tools-extra, so that does not make any difference.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109611

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

[PATCH] D87118: Add an explicit toggle for the static analyzer in clang-tidy

[Alexander Richardson via Phabricator via cfe-commits]

arichardson added inline comments.



Comment at: clang/lib/CMakeLists.txt:24
 add_subdirectory(IndexSerialization)
-if(CLANG_ENABLE_STATIC_ANALYZER)
-  add_subdirectory(StaticAnalyzer)

arichardson wrote:
> thakis wrote:
> > hans wrote:
> > > Why does removing the condition here work?
> > As far as I understand, it just impacts which targets CMake generates. 
> > clang/lib/FrontendTool/CMakeLists.txt only adds the dep on 
> > clangStaticAnalyzerFrontend if CLANG_ENABLE_STATIC_ANALYZER is set, so this 
> > doesn't change what gets built for "clang". If you build all targets, this 
> > will now always build the analyzer sources and I suppose it makes it a bit 
> > easier to accidentally depend on clangStaticAnalyzerFrontend , but I don't 
> > know of another way to be able to link this into clang-tidy when it's not 
> > built at all over here.
> I just noticed that my builds (just a plain `ninja`) are compiling all static 
> analyzer sources. I am explicitly passing 
> `-DCLANG_ENABLE_STATIC_ANALYZER=OFF` to cmake (and not building any 
> clang-tools-extra).
> I feel like this was not happening before so it's possible there was some 
> CMake change more recently that is now causing this behaviour.
> 
> I tried setting EXCLUDE_FROM_ALL 
> (https://cmake.org/cmake/help/latest/prop_tgt/EXCLUDE_FROM_ALL.html) on the 
> directories and targets but that didn't fix the issue for me.
> 
> How about changing the condition to
> `if (CLANG_TIDY_ENABLE_STATIC_ANALYZER OR CLANG_ENABLE_STATIC_ANALYZER)`? Or 
> will that not work since CLANG_TIDY_ENABLE_STATIC_ANALYZER isn't defined yet?
Possible solution: D109611


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

https://reviews.llvm.org/D87118

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

[PATCH] D109611: Fix CLANG_ENABLE_STATIC_ANALYZER=OFF building all analyzer source

[Alexander Richardson via Phabricator via cfe-commits]

arichardson created this revision.
arichardson added reviewers: thakis, hans, tstellar.
Herald added subscribers: manas, steakhal, ASDenysPetrov, martong, dkrupp, 
donat.nagy, Szelethus, a.sidorin, baloghadamsoftware, mgorny.
arichardson requested review of this revision.
Herald added projects: clang, LLVM.
Herald added subscribers: llvm-commits, cfe-commits.

Since https://reviews.llvm.org/D87118, the StaticAnalyzer directory is
added unconditionally. In theory this should not cause the static analyzer
sources to be built unless they are referenced by another target. However,
the clang-cpp target (defined in clang/tools/clang-shlib) uses the
CLANG_STATIC_LIBS global property to determine which libraries need to
be included. To solve this issue, this patch avoids adding libraries to
that property if EXCLUDE_FROM_ALL is set.

In case something like this comes up again: `cmake --graphviz=targets.dot`
is quite useful to see why a target is included as part of `ninja all`.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D109611

Files:
  clang/cmake/modules/AddClang.cmake
  clang/lib/StaticAnalyzer/CMakeLists.txt
  llvm/cmake/modules/AddLLVM.cmake


Index: llvm/cmake/modules/AddLLVM.cmake
===
--- llvm/cmake/modules/AddLLVM.cmake
+++ llvm/cmake/modules/AddLLVM.cmake
@@ -429,10 +429,13 @@
 #  This is used to specify that this is a component library of
 #  LLVM which means that the source resides in llvm/lib/ and it is a
 #  candidate for inclusion into libLLVM.so.
+#   EXCLUDE_FROM_ALL
+#  Do not build this library as part of the default target, only
+#  if explicitly requested or when linked against.
 #   )
 function(llvm_add_library name)
   cmake_parse_arguments(ARG
-
"MODULE;SHARED;STATIC;OBJECT;DISABLE_LLVM_LINK_LLVM_DYLIB;SONAME;NO_INSTALL_RPATH;COMPONENT_LIB"
+
"MODULE;SHARED;STATIC;OBJECT;DISABLE_LLVM_LINK_LLVM_DYLIB;SONAME;NO_INSTALL_RPATH;COMPONENT_LIB;EXCLUDE_FROM_ALL"
 "OUTPUT_NAME;PLUGIN_TOOL;ENTITLEMENTS;BUNDLE_PATH"
 "ADDITIONAL_HEADERS;DEPENDS;LINK_COMPONENTS;LINK_LIBS;OBJLIBS"
 ${ARGN})
@@ -535,6 +538,9 @@
 
 # FIXME: Add name_static to anywhere in TARGET ${name}'s PROPERTY.
 set(ARG_STATIC)
+if(ARG_EXCLUDE_FROM_ALL OR EXCLUDE_FROM_ALL)
+  set_target_properties(${name_static} PROPERTIES EXCLUDE_FROM_ALL ON)
+endif()
   endif()
 
   if(ARG_MODULE)
@@ -546,6 +552,10 @@
 add_library(${name} STATIC ${ALL_FILES})
   endif()
 
+  if(ARG_EXCLUDE_FROM_ALL OR EXCLUDE_FROM_ALL)
+set_target_properties(${name} PROPERTIES EXCLUDE_FROM_ALL ON)
+  endif()
+
   if(ARG_COMPONENT_LIB)
 set_target_properties(${name} PROPERTIES LLVM_COMPONENT TRUE)
 set_property(GLOBAL APPEND PROPERTY LLVM_COMPONENT_LIBS ${name})
Index: clang/lib/StaticAnalyzer/CMakeLists.txt
===
--- clang/lib/StaticAnalyzer/CMakeLists.txt
+++ clang/lib/StaticAnalyzer/CMakeLists.txt
@@ -1,3 +1,10 @@
+# These directories can significantly impact build time, only build
+# them if anything depends on the clangStaticAnalyzer* libraries.
+if(NOT CLANG_ENABLE_STATIC_ANALYZER)
+  set_property(DIRECTORY PROPERTY EXCLUDE_FROM_ALL ON)
+  set(EXCLUDE_FROM_ALL ON)
+endif()
+
 add_subdirectory(Core)
 add_subdirectory(Checkers)
 add_subdirectory(Frontend)
Index: clang/cmake/modules/AddClang.cmake
===
--- clang/cmake/modules/AddClang.cmake
+++ clang/cmake/modules/AddClang.cmake
@@ -100,7 +100,12 @@
   # The Xcode generator doesn't handle object libraries correctly.
   list(APPEND LIBTYPE OBJECT)
 endif()
-set_property(GLOBAL APPEND PROPERTY CLANG_STATIC_LIBS ${name})
+if (NOT EXCLUDE_FROM_ALL)
+  # Only include libraries that don't have EXCLUDE_FROM_ALL set. This
+  # ensure that the clang static analyzer libraries are not compiled
+  # as part of clang-shlib if CLANG_ENABLE_STATIC_ANALYZER=OFF.
+  set_property(GLOBAL APPEND PROPERTY CLANG_STATIC_LIBS ${name})
+endif()
   endif()
   llvm_add_library(${name} ${LIBTYPE} ${ARG_UNPARSED_ARGUMENTS} ${srcs})
 
@@ -110,6 +115,9 @@
   endif()
 
   foreach(lib ${libs})
+if (EXCLUDE_FROM_ALL)
+  continue()
+endif()
 if(TARGET ${lib})
   target_link_libraries(${lib} INTERFACE ${LLVM_COMMON_LIBS})
 


Index: llvm/cmake/modules/AddLLVM.cmake
===
--- llvm/cmake/modules/AddLLVM.cmake
+++ llvm/cmake/modules/AddLLVM.cmake
@@ -429,10 +429,13 @@
 #  This is used to specify that this is a component library of
 #  LLVM which means that the source resides in llvm/lib/ and it is a
 #  candidate for inclusion into libLLVM.so.
+#   EXCLUDE_FROM_ALL
+#  Do not build this library as part of the default target, only
+#  if explicitly requested or when linked against.
 #   )
 function(llvm_add_library name)
   

[PATCH] D87118: Add an explicit toggle for the static analyzer in clang-tidy

[Alexander Richardson via Phabricator via cfe-commits]

arichardson added inline comments.
Herald added subscribers: manas, steakhal.
Herald added a project: clang-tools-extra.



Comment at: clang/lib/CMakeLists.txt:24
 add_subdirectory(IndexSerialization)
-if(CLANG_ENABLE_STATIC_ANALYZER)
-  add_subdirectory(StaticAnalyzer)

thakis wrote:
> hans wrote:
> > Why does removing the condition here work?
> As far as I understand, it just impacts which targets CMake generates. 
> clang/lib/FrontendTool/CMakeLists.txt only adds the dep on 
> clangStaticAnalyzerFrontend if CLANG_ENABLE_STATIC_ANALYZER is set, so this 
> doesn't change what gets built for "clang". If you build all targets, this 
> will now always build the analyzer sources and I suppose it makes it a bit 
> easier to accidentally depend on clangStaticAnalyzerFrontend , but I don't 
> know of another way to be able to link this into clang-tidy when it's not 
> built at all over here.
I just noticed that my builds (just a plain `ninja`) are compiling all static 
analyzer sources. I am explicitly passing `-DCLANG_ENABLE_STATIC_ANALYZER=OFF` 
to cmake (and not building any clang-tools-extra).
I feel like this was not happening before so it's possible there was some CMake 
change more recently that is now causing this behaviour.

I tried setting EXCLUDE_FROM_ALL 
(https://cmake.org/cmake/help/latest/prop_tgt/EXCLUDE_FROM_ALL.html) on the 
directories and targets but that didn't fix the issue for me.

How about changing the condition to
`if (CLANG_TIDY_ENABLE_STATIC_ANALYZER OR CLANG_ENABLE_STATIC_ANALYZER)`? Or 
will that not work since CLANG_TIDY_ENABLE_STATIC_ANALYZER isn't defined yet?


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

https://reviews.llvm.org/D87118

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

[PATCH] D105972: Fix __attribute__((annotate("")) with non-zero globals AS

[Alexander Richardson via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG7cab90a7b1c4: Fix __attribute__((annotate()) 
with non-zero globals AS (authored by arichardson).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105972

Files:
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/CodeGen/CodeGenTypeCache.h
  clang/test/CodeGen/annotations-global.c


Index: clang/test/CodeGen/annotations-global.c
===
--- clang/test/CodeGen/annotations-global.c
+++ clang/test/CodeGen/annotations-global.c
@@ -4,6 +4,7 @@
 // RUN: FileCheck --check-prefix=BAR %s < %t1
 // RUN: FileCheck --check-prefix=FOOS %s < %t1
 // RUN: FileCheck --check-prefix=ADDRSPACE %s < %t1
+// RUN: %clang_cc1 %s -triple r600 -emit-llvm -o - | FileCheck %s 
--check-prefix AS1-GLOBALS
 // END.
 
 static __attribute((annotate("sfoo_0"))) __attribute((annotate("sfoo_1"))) 
char sfoo;
@@ -45,3 +46,8 @@
 
 // ADDRSPACE: target triple
 // ADDRSPACE: @llvm.global.annotations = appending global {{.*}} addrspacecast 
(i8 addrspace(1)* @addrspace1_var to i8*), {{.*}}
+
+// AS1-GLOBALS: target datalayout = "{{.+}}-A5-G1"
+// AS1-GLOBALS: @llvm.global.annotations = appending addrspace(1) global [11 x 
{ i8 addrspace(1)*, i8 addrspace(1)*, i8 addrspace(1)*, i32, i8 addrspace(1)* }]
+// AS1-GLOBALS-SAME: { i8 addrspace(1)* @a.bar,
+// AS1-GLOBALS-SAME: { i8 addrspace(1)* @addrspace1_var,
Index: clang/lib/CodeGen/CodeGenTypeCache.h
===
--- clang/lib/CodeGen/CodeGenTypeCache.h
+++ clang/lib/CodeGen/CodeGenTypeCache.h
@@ -69,6 +69,12 @@
 llvm::PointerType *AllocaInt8PtrTy;
   };
 
+  /// void* in default globals address space
+  union {
+llvm::PointerType *GlobalsVoidPtrTy;
+llvm::PointerType *GlobalsInt8PtrTy;
+  };
+
   /// The size and alignment of the builtin C type 'int'.  This comes
   /// up enough in various ABI lowering tasks to be worth pre-computing.
   union {
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -130,8 +130,9 @@
 C.getTargetInfo().getMaxPointerWidth());
   Int8PtrTy = Int8Ty->getPointerTo(0);
   Int8PtrPtrTy = Int8PtrTy->getPointerTo(0);
-  AllocaInt8PtrTy = Int8Ty->getPointerTo(
-  M.getDataLayout().getAllocaAddrSpace());
+  const llvm::DataLayout  = M.getDataLayout();
+  AllocaInt8PtrTy = Int8Ty->getPointerTo(DL.getAllocaAddrSpace());
+  GlobalsInt8PtrTy = Int8Ty->getPointerTo(DL.getDefaultGlobalsAddressSpace());
   ASTAllocaAddressSpace = getTargetCodeGenInfo().getASTAllocaAddressSpace();
 
   RuntimeCC = getTargetCodeGenInfo().getABIInfo().getRuntimeCC();
@@ -2532,7 +2533,7 @@
 llvm::Constant *CodeGenModule::EmitAnnotationArgs(const AnnotateAttr *Attr) {
   ArrayRef Exprs = {Attr->args_begin(), Attr->args_size()};
   if (Exprs.empty())
-return llvm::ConstantPointerNull::get(Int8PtrTy);
+return llvm::ConstantPointerNull::get(GlobalsInt8PtrTy);
 
   llvm::FoldingSetNodeID ID;
   for (Expr *E : Exprs) {
@@ -2556,7 +2557,7 @@
   ".args");
   GV->setSection(AnnotationSection);
   GV->setUnnamedAddr(llvm::GlobalValue::UnnamedAddr::Global);
-  auto *Bitcasted = llvm::ConstantExpr::getBitCast(GV, Int8PtrTy);
+  auto *Bitcasted = llvm::ConstantExpr::getBitCast(GV, GlobalsInt8PtrTy);
 
   Lookup = Bitcasted;
   return Bitcasted;
@@ -2571,17 +2572,19 @@
  *LineNoCst = EmitAnnotationLineNo(L),
  *Args = EmitAnnotationArgs(AA);
 
-  llvm::Constant *ASZeroGV = GV;
-  if (GV->getAddressSpace() != 0) {
-ASZeroGV = llvm::ConstantExpr::getAddrSpaceCast(
-   GV, GV->getValueType()->getPointerTo(0));
+  llvm::Constant *GVInGlobalsAS = GV;
+  if (GV->getAddressSpace() !=
+  getDataLayout().getDefaultGlobalsAddressSpace()) {
+GVInGlobalsAS = llvm::ConstantExpr::getAddrSpaceCast(
+GV, GV->getValueType()->getPointerTo(
+getDataLayout().getDefaultGlobalsAddressSpace()));
   }
 
   // Create the ConstantStruct for the global annotation.
   llvm::Constant *Fields[] = {
-  llvm::ConstantExpr::getBitCast(ASZeroGV, Int8PtrTy),
-  llvm::ConstantExpr::getBitCast(AnnoGV, Int8PtrTy),
-  llvm::ConstantExpr::getBitCast(UnitGV, Int8PtrTy),
+  llvm::ConstantExpr::getBitCast(GVInGlobalsAS, GlobalsInt8PtrTy),
+  llvm::ConstantExpr::getBitCast(AnnoGV, GlobalsInt8PtrTy),
+  llvm::ConstantExpr::getBitCast(UnitGV, GlobalsInt8PtrTy),
   LineNoCst,
   Args,
   };


Index: clang/test/CodeGen/annotations-global.c
===
--- clang/test/CodeGen/annotations-global.c
+++ clang/test/CodeGen/annotations-global.c
@@ -4,6 +4,7 @@
 // RUN: FileCheck --check-prefix=BAR %s < %t1
 // 

[PATCH] D108110: Fix LLVM_ENABLE_THREADS check from 26a92d5852b2c6bf77efd26f6c0194c913f40285

[Alexander Richardson via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGbf66b0eefcda: Fix LLVM_ENABLE_THREADS check from 
26a92d5852b2c6bf77efd26f6c0194c913f40285 (authored by arichardson).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108110

Files:
  clang/include/clang/Basic/Stack.h


Index: clang/include/clang/Basic/Stack.h
===
--- clang/include/clang/Basic/Stack.h
+++ clang/include/clang/Basic/Stack.h
@@ -39,7 +39,7 @@
   /// is insufficient, calls Diag to emit a diagnostic before calling Fn.
   inline void runWithSufficientStackSpace(llvm::function_ref Diag,
   llvm::function_ref Fn) {
-#ifdef LLVM_ENABLE_THREADS
+#if LLVM_ENABLE_THREADS
 if (LLVM_UNLIKELY(isStackNearlyExhausted()))
   runWithSufficientStackSpaceSlow(Diag, Fn);
 else


Index: clang/include/clang/Basic/Stack.h
===
--- clang/include/clang/Basic/Stack.h
+++ clang/include/clang/Basic/Stack.h
@@ -39,7 +39,7 @@
   /// is insufficient, calls Diag to emit a diagnostic before calling Fn.
   inline void runWithSufficientStackSpace(llvm::function_ref Diag,
   llvm::function_ref Fn) {
-#ifdef LLVM_ENABLE_THREADS
+#if LLVM_ENABLE_THREADS
 if (LLVM_UNLIKELY(isStackNearlyExhausted()))
   runWithSufficientStackSpaceSlow(Diag, Fn);
 else
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D105972: Fix __attribute__((annotate("")) with non-zero globals AS

[Alexander Richardson via Phabricator via cfe-commits]

arichardson updated this revision to Diff 366911.
arichardson added a comment.

rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105972

Files:
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/CodeGen/CodeGenTypeCache.h
  clang/test/CodeGen/annotations-global.c


Index: clang/test/CodeGen/annotations-global.c
===
--- clang/test/CodeGen/annotations-global.c
+++ clang/test/CodeGen/annotations-global.c
@@ -4,6 +4,7 @@
 // RUN: FileCheck --check-prefix=BAR %s < %t1
 // RUN: FileCheck --check-prefix=FOOS %s < %t1
 // RUN: FileCheck --check-prefix=ADDRSPACE %s < %t1
+// RUN: %clang_cc1 %s -triple r600 -emit-llvm -o - | FileCheck %s 
--check-prefix AS1-GLOBALS
 // END.
 
 static __attribute((annotate("sfoo_0"))) __attribute((annotate("sfoo_1"))) 
char sfoo;
@@ -45,3 +46,8 @@
 
 // ADDRSPACE: target triple
 // ADDRSPACE: @llvm.global.annotations = appending global {{.*}} addrspacecast 
(i8 addrspace(1)* @addrspace1_var to i8*), {{.*}}
+
+// AS1-GLOBALS: target datalayout = "{{.+}}-A5-G1"
+// AS1-GLOBALS: @llvm.global.annotations = appending addrspace(1) global [11 x 
{ i8 addrspace(1)*, i8 addrspace(1)*, i8 addrspace(1)*, i32, i8 addrspace(1)* }]
+// AS1-GLOBALS-SAME: { i8 addrspace(1)* @a.bar,
+// AS1-GLOBALS-SAME: { i8 addrspace(1)* @addrspace1_var,
Index: clang/lib/CodeGen/CodeGenTypeCache.h
===
--- clang/lib/CodeGen/CodeGenTypeCache.h
+++ clang/lib/CodeGen/CodeGenTypeCache.h
@@ -69,6 +69,12 @@
 llvm::PointerType *AllocaInt8PtrTy;
   };
 
+  /// void* in default globals address space
+  union {
+llvm::PointerType *GlobalsVoidPtrTy;
+llvm::PointerType *GlobalsInt8PtrTy;
+  };
+
   /// The size and alignment of the builtin C type 'int'.  This comes
   /// up enough in various ABI lowering tasks to be worth pre-computing.
   union {
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -130,8 +130,9 @@
 C.getTargetInfo().getMaxPointerWidth());
   Int8PtrTy = Int8Ty->getPointerTo(0);
   Int8PtrPtrTy = Int8PtrTy->getPointerTo(0);
-  AllocaInt8PtrTy = Int8Ty->getPointerTo(
-  M.getDataLayout().getAllocaAddrSpace());
+  const llvm::DataLayout  = M.getDataLayout();
+  AllocaInt8PtrTy = Int8Ty->getPointerTo(DL.getAllocaAddrSpace());
+  GlobalsInt8PtrTy = Int8Ty->getPointerTo(DL.getDefaultGlobalsAddressSpace());
   ASTAllocaAddressSpace = getTargetCodeGenInfo().getASTAllocaAddressSpace();
 
   RuntimeCC = getTargetCodeGenInfo().getABIInfo().getRuntimeCC();
@@ -2531,7 +2532,7 @@
 llvm::Constant *CodeGenModule::EmitAnnotationArgs(const AnnotateAttr *Attr) {
   ArrayRef Exprs = {Attr->args_begin(), Attr->args_size()};
   if (Exprs.empty())
-return llvm::ConstantPointerNull::get(Int8PtrTy);
+return llvm::ConstantPointerNull::get(GlobalsInt8PtrTy);
 
   llvm::FoldingSetNodeID ID;
   for (Expr *E : Exprs) {
@@ -2555,7 +2556,7 @@
   ".args");
   GV->setSection(AnnotationSection);
   GV->setUnnamedAddr(llvm::GlobalValue::UnnamedAddr::Global);
-  auto *Bitcasted = llvm::ConstantExpr::getBitCast(GV, Int8PtrTy);
+  auto *Bitcasted = llvm::ConstantExpr::getBitCast(GV, GlobalsInt8PtrTy);
 
   Lookup = Bitcasted;
   return Bitcasted;
@@ -2570,17 +2571,19 @@
  *LineNoCst = EmitAnnotationLineNo(L),
  *Args = EmitAnnotationArgs(AA);
 
-  llvm::Constant *ASZeroGV = GV;
-  if (GV->getAddressSpace() != 0) {
-ASZeroGV = llvm::ConstantExpr::getAddrSpaceCast(
-   GV, GV->getValueType()->getPointerTo(0));
+  llvm::Constant *GVInGlobalsAS = GV;
+  if (GV->getAddressSpace() !=
+  getDataLayout().getDefaultGlobalsAddressSpace()) {
+GVInGlobalsAS = llvm::ConstantExpr::getAddrSpaceCast(
+GV, GV->getValueType()->getPointerTo(
+getDataLayout().getDefaultGlobalsAddressSpace()));
   }
 
   // Create the ConstantStruct for the global annotation.
   llvm::Constant *Fields[] = {
-  llvm::ConstantExpr::getBitCast(ASZeroGV, Int8PtrTy),
-  llvm::ConstantExpr::getBitCast(AnnoGV, Int8PtrTy),
-  llvm::ConstantExpr::getBitCast(UnitGV, Int8PtrTy),
+  llvm::ConstantExpr::getBitCast(GVInGlobalsAS, GlobalsInt8PtrTy),
+  llvm::ConstantExpr::getBitCast(AnnoGV, GlobalsInt8PtrTy),
+  llvm::ConstantExpr::getBitCast(UnitGV, GlobalsInt8PtrTy),
   LineNoCst,
   Args,
   };


Index: clang/test/CodeGen/annotations-global.c
===
--- clang/test/CodeGen/annotations-global.c
+++ clang/test/CodeGen/annotations-global.c
@@ -4,6 +4,7 @@
 // RUN: FileCheck --check-prefix=BAR %s < %t1
 // RUN: FileCheck --check-prefix=FOOS %s < %t1
 // RUN: FileCheck --check-prefix=ADDRSPACE %s < %t1
+// RUN: 

[PATCH] D108212: Emit offsetof values as notes when a static_assert fails

[Alexander Richardson via Phabricator via cfe-commits]

arichardson created this revision.
arichardson added reviewers: courbet, Quuxplusone, aaron.ballman.
arichardson requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Trying to debug a static_assert(offset(foo, field) == ...) failure can be
rather awkward since there is no easy way to print the value at compile
time without another compiler diagnostic involving an array size.
This builds upon the sizeof()/alignof() printing and extends it to handle
OffsetOfExpr.

Depends on D108211 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D108212

Files:
  clang/lib/Sema/SemaTemplate.cpp
  clang/test/SemaCXX/static-assert.cpp


Index: clang/test/SemaCXX/static-assert.cpp
===
--- clang/test/SemaCXX/static-assert.cpp
+++ clang/test/SemaCXX/static-assert.cpp
@@ -216,3 +216,10 @@
 // expected-error@-1{{static_assert failed due to requirement 
'alignof(IntAndPointer) == sizeof(IntAndPointer)' "message"}}
 // expected-note@-2{{with 'alignof(IntAndPointer)' equal to 8}}
 // expected-note@-3{{with 'sizeof(IntAndPointer)' equal to 16}}
+#define offsetof(s, f) __builtin_offsetof(s, f)
+static_assert(__builtin_offsetof(IntAndPointer, p) == -1, "message");
+// expected-error@-1{{static_assert failed due to requirement 
'__builtin_offsetof(IntAndPointer, p) == -1' "message"}}
+// expected-note@-2{{with '__builtin_offsetof(IntAndPointer, p)' equal to 8}}
+static_assert(offsetof(IntAndPointer, i) == -1, "message");
+// expected-error@-1{{static_assert failed due to requirement 
'__builtin_offsetof(IntAndPointer, i) == -1' "message"}}
+// expected-note@-2{{with '__builtin_offsetof(IntAndPointer, i)' equal to 0}}
Index: clang/lib/Sema/SemaTemplate.cpp
===
--- clang/lib/Sema/SemaTemplate.cpp
+++ clang/lib/Sema/SemaTemplate.cpp
@@ -3577,10 +3577,10 @@
   explicit FailedBooleanConditionPrinterHelper(
   const PrintingPolicy , Sema ,
   SmallVectorImpl )
-  : Policy(P), S(S), Notes(Notes) {}
+  : Policy(P), SemaRef(S), Notes(Notes) {}
 
-  bool handledStmt(Stmt *E, raw_ostream ) override {
-const auto *DR = dyn_cast(E);
+  bool handledStmt(Stmt *S, raw_ostream ) override {
+const auto *DR = dyn_cast(S);
 if (DR && DR->getQualifier()) {
   // If this is a qualified name, expand the template arguments in nested
   // qualifiers.
@@ -3595,18 +3595,20 @@
 IV->getSpecializedTemplate()->getTemplateParameters());
   }
   return true;
-} else if (auto *UE = dyn_cast(E)) {
+} else if (isa(S) || isa(S)) {
+  Expr *E = cast(S);
   Expr::EvalResult Result;
-  if (UE->EvaluateAsConstantExpr(Result, S.Context) && Result.Val.isInt()) 
{
+  if (E->EvaluateAsConstantExpr(Result, SemaRef.Context) &&
+  Result.Val.isInt()) {
 std::string ExprStr;
 llvm::raw_string_ostream ExprStream(ExprStr);
-UE->printPretty(ExprStream, nullptr, Policy);
+E->printPretty(ExprStream, nullptr, Policy);
 ExprStream.flush();
 Notes.push_back(PartialDiagnosticAt(
-UE->getExprLoc(),
-S.PDiag(diag::note_static_assert_requirement_context)
+E->getExprLoc(),
+SemaRef.PDiag(diag::note_static_assert_requirement_context)
 << ExprStr << toString(Result.Val.getInt(), 10)
-<< UE->getSourceRange()));
+<< E->getSourceRange()));
   }
 }
 return false;
@@ -3614,7 +3616,7 @@
 
 private:
   const PrintingPolicy Policy;
-  Sema 
+  Sema 
   SmallVectorImpl 
 };
 


Index: clang/test/SemaCXX/static-assert.cpp
===
--- clang/test/SemaCXX/static-assert.cpp
+++ clang/test/SemaCXX/static-assert.cpp
@@ -216,3 +216,10 @@
 // expected-error@-1{{static_assert failed due to requirement 'alignof(IntAndPointer) == sizeof(IntAndPointer)' "message"}}
 // expected-note@-2{{with 'alignof(IntAndPointer)' equal to 8}}
 // expected-note@-3{{with 'sizeof(IntAndPointer)' equal to 16}}
+#define offsetof(s, f) __builtin_offsetof(s, f)
+static_assert(__builtin_offsetof(IntAndPointer, p) == -1, "message");
+// expected-error@-1{{static_assert failed due to requirement '__builtin_offsetof(IntAndPointer, p) == -1' "message"}}
+// expected-note@-2{{with '__builtin_offsetof(IntAndPointer, p)' equal to 8}}
+static_assert(offsetof(IntAndPointer, i) == -1, "message");
+// expected-error@-1{{static_assert failed due to requirement '__builtin_offsetof(IntAndPointer, i) == -1' "message"}}
+// expected-note@-2{{with '__builtin_offsetof(IntAndPointer, i)' equal to 0}}
Index: clang/lib/Sema/SemaTemplate.cpp
===
--- clang/lib/Sema/SemaTemplate.cpp
+++ clang/lib/Sema/SemaTemplate.cpp
@@ -3577,10 +3577,10 @@
   explicit 

[PATCH] D108211: Emit sizeof/alignof values as notes when a static_assert fails

[Alexander Richardson via Phabricator via cfe-commits]

arichardson created this revision.
arichardson added reviewers: courbet, Quuxplusone, aaron.ballman.
arichardson requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Trying to debug a static_assert(sizeof(foo) == ...) failure can be rather
awkward since there is no easy way to print the value of the sizeof() at
compile-time without another compiler diagnostic involving an array size.
This patch emits additional notes when a static_assert fails for any
UnaryExprOrTypeTraitExpr expression inside the static_assert.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D108211

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/lib/Sema/SemaTemplate.cpp
  clang/test/CXX/dcl.decl/dcl.meaning/dcl.array/p3.cpp
  clang/test/Parser/objc-static-assert.mm
  clang/test/Sema/static-assert.c
  clang/test/SemaCXX/constant-expression-cxx11.cpp
  clang/test/SemaCXX/static-assert-cxx17.cpp
  clang/test/SemaCXX/static-assert.cpp
  clang/test/SemaTemplate/overload-candidates.cpp

Index: clang/test/SemaTemplate/overload-candidates.cpp
===
--- clang/test/SemaTemplate/overload-candidates.cpp
+++ clang/test/SemaTemplate/overload-candidates.cpp
@@ -62,6 +62,7 @@
 
 template struct NonTemplateFunction {
   typename boost::enable_if::type f(); // expected-error{{failed requirement 'sizeof(char) == 4'; 'enable_if' cannot be used to disable this declaration}}
+  // expected-note@-1{{with 'sizeof(char)' equal to 1}}
 };
 NonTemplateFunction NTFC; // expected-note{{here}}
 
Index: clang/test/SemaCXX/static-assert.cpp
===
--- clang/test/SemaCXX/static-assert.cpp
+++ clang/test/SemaCXX/static-assert.cpp
@@ -21,8 +21,10 @@
 T<1> t1; // expected-note {{in instantiation of template class 'T<1>' requested here}}
 T<2> t2;
 
-template struct S {
-static_assert(sizeof(T) > sizeof(char), "Type not big enough!"); // expected-error {{static_assert failed due to requirement 'sizeof(char) > sizeof(char)' "Type not big enough!"}}
+template  struct S {
+  static_assert(sizeof(T) > sizeof(char), "Type not big enough!"); // expected-error {{static_assert failed due to requirement 'sizeof(char) > sizeof(char)' "Type not big enough!"}}
+   // expected-note@-1{{with 'sizeof(char)' equal to 1}}
+   // expected-note@-2{{with 'sizeof(char)' equal to 1}}
 };
 
 S s1; // expected-note {{in instantiation of template class 'S' requested here}}
@@ -199,3 +201,18 @@
 constexpr NotBool constexprNotBool;
 static_assert(notBool, "message");  // expected-error {{value of type 'struct NotBool' is not contextually convertible to 'bool'}}
 static_assert(constexprNotBool, "message"); // expected-error {{value of type 'const NotBool' is not contextually convertible to 'bool'}}
+
+struct IntAndPointer {
+  int i;
+  void *p;
+};
+static_assert(sizeof(IntAndPointer) == 4, "message");
+// expected-error@-1{{static_assert failed due to requirement 'sizeof(IntAndPointer) == 4' "message"}}
+// expected-note@-2{{with 'sizeof(IntAndPointer)' equal to 16}}
+static_assert(alignof(IntAndPointer) == 4, "message");
+// expected-error@-1{{static_assert failed due to requirement 'alignof(IntAndPointer) == 4' "message"}}
+// expected-note@-2{{with 'alignof(IntAndPointer)' equal to 8}}
+static_assert(alignof(IntAndPointer) == sizeof(IntAndPointer), "message");
+// expected-error@-1{{static_assert failed due to requirement 'alignof(IntAndPointer) == sizeof(IntAndPointer)' "message"}}
+// expected-note@-2{{with 'alignof(IntAndPointer)' equal to 8}}
+// expected-note@-3{{with 'sizeof(IntAndPointer)' equal to 16}}
Index: clang/test/SemaCXX/static-assert-cxx17.cpp
===
--- clang/test/SemaCXX/static-assert-cxx17.cpp
+++ clang/test/SemaCXX/static-assert-cxx17.cpp
@@ -89,6 +89,7 @@
   // expected-error@-1{{static_assert failed due to requirement 'int(0)'}}
   static_assert(sizeof(X) == 0);
   // expected-error@-1{{static_assert failed due to requirement 'sizeof(X) == 0'}}
+  // expected-note@-2{{with 'sizeof(X)' equal to 8}}
   static_assert((const X *)nullptr);
   // expected-error@-1{{static_assert failed due to requirement '(const X *)nullptr'}}
   static_assert(static_cast *>(nullptr));
@@ -97,6 +98,7 @@
   // expected-error@-1{{static_assert failed due to requirement '(X const[0]){} == nullptr'}}
   static_assert(sizeof(X().X::~X())>) == 0);
   // expected-error@-1{{static_assert failed due to requirement 'sizeof(X) == 0'}}
+  // expected-note@-2{{with 'sizeof(X)' equal to 8}}
   static_assert(constexpr_return_false());
   // expected-error@-1{{static_assert failed due to requirement 'constexpr_return_false()'}}
 }
Index: 

[PATCH] D108110: Fix LLVM_ENABLE_THREADS check from 26a92d5852b2c6bf77efd26f6c0194c913f40285

[Alexander Richardson via Phabricator via cfe-commits]

arichardson created this revision.
arichardson added reviewers: aaron.ballman, rsmith.
Herald added a subscriber: dexonsmith.
arichardson requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

We should be using #if instead of #ifdef here since LLVM_ENABLE_THREADS
is set using #cmakedefine01 so is always defined.

Since the other branch has never been used I wonder if we should just remove it 
instead?


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D108110

Files:
  clang/include/clang/Basic/Stack.h


Index: clang/include/clang/Basic/Stack.h
===
--- clang/include/clang/Basic/Stack.h
+++ clang/include/clang/Basic/Stack.h
@@ -39,7 +39,7 @@
   /// is insufficient, calls Diag to emit a diagnostic before calling Fn.
   inline void runWithSufficientStackSpace(llvm::function_ref Diag,
   llvm::function_ref Fn) {
-#ifdef LLVM_ENABLE_THREADS
+#if LLVM_ENABLE_THREADS
 if (LLVM_UNLIKELY(isStackNearlyExhausted()))
   runWithSufficientStackSpaceSlow(Diag, Fn);
 else


Index: clang/include/clang/Basic/Stack.h
===
--- clang/include/clang/Basic/Stack.h
+++ clang/include/clang/Basic/Stack.h
@@ -39,7 +39,7 @@
   /// is insufficient, calls Diag to emit a diagnostic before calling Fn.
   inline void runWithSufficientStackSpace(llvm::function_ref Diag,
   llvm::function_ref Fn) {
-#ifdef LLVM_ENABLE_THREADS
+#if LLVM_ENABLE_THREADS
 if (LLVM_UNLIKELY(isStackNearlyExhausted()))
   runWithSufficientStackSpaceSlow(Diag, Fn);
 else
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D95583: Frontend: Add -f{, no-}implicit-modules-uses-lock and -Rmodule-lock

[Alexander Richardson via Phabricator via cfe-commits]

arichardson added inline comments.



Comment at: clang/test/Modules/implicit-modules-use-lock.m:21
+
+// CHECK-NO-LOCKS-NOT: remark:
+// CHECK-LOCKS: remark: locking '{{.*}}.pcm' to build module 'X' 
[-Rmodule-lock]

jansvoboda11 wrote:
> dexonsmith wrote:
> > jansvoboda11 wrote:
> > > Where is the empty remark coming from? Is it useful to us in any way?
> > This is a `CHECK-NOT: ` line (with a custom `-check-prefix`). It 
> > confirms that `` does not occur at least until the next positive 
> > check matches. The `FileCheck` invocation that uses `CHECK-NO-LOCKS` has no 
> > other check lines, so this really means: "`remark:` does not match on any 
> > line". Note I also needed to add `-allow-empty` since with no diagnostics 
> > at all, `FileCheck`'s stdin will be empty.
> > 
> > FYI, if you want to read more:
> > - `-check-prefix` is documented at 
> > https://www.llvm.org/docs/CommandGuide/FileCheck.html#the-filecheck-check-prefix-option
> > - `CHECK-NOT` is documented at 
> > https://www.llvm.org/docs/CommandGuide/FileCheck.html#the-check-not-directive
> > 
> > Often in clang tests it's easier to use `-cc1 -verify` for diagnostics 
> > instead of manual `FileCheck`s (`expected-no-diagnostics` comment being the 
> > equivalent of `-allow-empty`). In the modules testcases, there's often a 
> > complicated setup that we want to run a lot of `RUN:` lines against where 
> > each one expects different diagnostics. Maybe we should add prefix support 
> > to `-verify` (or maybe it's there and no one told me...).
> Ah, that makes sense, thanks!
This could use e.g. `-verify=lock` and `-verify=lock` to avoid the Filecheck 
usage.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95583

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

[PATCH] D105516: [clang][PassManager] Add -falways-mem2reg to run mem2reg at -O0

[Alexander Richardson via Phabricator via cfe-commits]

arichardson added a comment.

I like this since the flag significantly improves readability for 
`update_cc_test_checks.py`-generated Clang test without having to use the 
`-disable-O0-optnone | opt` trick. Not sure what the best flag name is, but as 
long as it's a CC1 flag it shouldn't really matter.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105516

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

[PATCH] D105972: Fix __attribute__((annotate("")) with non-zero globals AS

[Alexander Richardson via Phabricator via cfe-commits]

arichardson added a comment.

ping?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105972

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

[PATCH] D105555: [PoC][RISCV][Clang] Compute the default target-abi if it's empty.

[Alexander Richardson via Phabricator via cfe-commits]

arichardson added inline comments.



Comment at: llvm/include/llvm/Support/TargetParser.h:177
 StringRef resolveTuneCPUAlias(StringRef TuneCPU, bool IsRV64);
+StringRef computeABIByArch(bool HasD, bool HasE, bool IsRV64);
 

Maybe this should be `computeDefaultABIFromArch()`? If `llvm::RISCVISAInfo` can 
be used here passing `const llvm::RISCVISAInfo&` to the function would also 
avoid some duplicated lines.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D10

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

[PATCH] D106243: [Utils] Support class template specializations in update_cc_test_checks

[Alexander Richardson via Phabricator via cfe-commits]

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

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106243

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

[PATCH] D105972: Fix __attribute__((annotate("")) with non-zero globals AS

[Alexander Richardson via Phabricator via cfe-commits]

arichardson created this revision.
arichardson added reviewers: rjmccall, nhaehnle, Tyker.
Herald added subscribers: jrtc27, luismarques, s.egerton, PkmX, simoncook, tpr.
arichardson requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

The existing code attempting to bitcast from a value in the default
globals AS to i8 addrspace(0)* was triggering an assertion failure for me.
I found this while compiling poppler for CHERI-RISC-V (we use AS200 for
all globals). The test case uses AMDGPU since that is one of the in-tree
targets with a non-zero default globals address space.
The new test previously triggered a "Invalid constantexpr bitcast!"
assertion an now correctly generates code with addrspace(1) pointers.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D105972

Files:
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/CodeGen/CodeGenTypeCache.h
  clang/test/CodeGen/annotations-global.c


Index: clang/test/CodeGen/annotations-global.c
===
--- clang/test/CodeGen/annotations-global.c
+++ clang/test/CodeGen/annotations-global.c
@@ -4,6 +4,7 @@
 // RUN: FileCheck --check-prefix=BAR %s < %t1
 // RUN: FileCheck --check-prefix=FOOS %s < %t1
 // RUN: FileCheck --check-prefix=ADDRSPACE %s < %t1
+// RUN: %clang_cc1 %s -triple r600 -emit-llvm -o - | FileCheck %s 
--check-prefix AS1-GLOBALS
 // END.
 
 static __attribute((annotate("sfoo_0"))) __attribute((annotate("sfoo_1"))) 
char sfoo;
@@ -45,3 +46,8 @@
 
 // ADDRSPACE: target triple
 // ADDRSPACE: @llvm.global.annotations = appending global {{.*}} addrspacecast 
(i8 addrspace(1)* @addrspace1_var to i8*), {{.*}}
+
+// AS1-GLOBALS: target datalayout = "{{.+}}-A5-G1"
+// AS1-GLOBALS: @llvm.global.annotations = appending addrspace(1) global [11 x 
{ i8 addrspace(1)*, i8 addrspace(1)*, i8 addrspace(1)*, i32, i8 addrspace(1)* }]
+// AS1-GLOBALS-SAME: { i8 addrspace(1)* @a.bar,
+// AS1-GLOBALS-SAME: { i8 addrspace(1)* @addrspace1_var,
Index: clang/lib/CodeGen/CodeGenTypeCache.h
===
--- clang/lib/CodeGen/CodeGenTypeCache.h
+++ clang/lib/CodeGen/CodeGenTypeCache.h
@@ -69,6 +69,12 @@
 llvm::PointerType *AllocaInt8PtrTy;
   };
 
+  /// void* in default globals address space
+  union {
+llvm::PointerType *GlobalsVoidPtrTy;
+llvm::PointerType *GlobalsInt8PtrTy;
+  };
+
   /// The size and alignment of the builtin C type 'int'.  This comes
   /// up enough in various ABI lowering tasks to be worth pre-computing.
   union {
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -130,8 +130,9 @@
 C.getTargetInfo().getMaxPointerWidth());
   Int8PtrTy = Int8Ty->getPointerTo(0);
   Int8PtrPtrTy = Int8PtrTy->getPointerTo(0);
-  AllocaInt8PtrTy = Int8Ty->getPointerTo(
-  M.getDataLayout().getAllocaAddrSpace());
+  const llvm::DataLayout  = M.getDataLayout();
+  AllocaInt8PtrTy = Int8Ty->getPointerTo(DL.getAllocaAddrSpace());
+  GlobalsInt8PtrTy = Int8Ty->getPointerTo(DL.getDefaultGlobalsAddressSpace());
   ASTAllocaAddressSpace = getTargetCodeGenInfo().getASTAllocaAddressSpace();
 
   RuntimeCC = getTargetCodeGenInfo().getABIInfo().getRuntimeCC();
@@ -2515,7 +2516,7 @@
 llvm::Constant *CodeGenModule::EmitAnnotationArgs(const AnnotateAttr *Attr) {
   ArrayRef Exprs = {Attr->args_begin(), Attr->args_size()};
   if (Exprs.empty())
-return llvm::ConstantPointerNull::get(Int8PtrTy);
+return llvm::ConstantPointerNull::get(GlobalsInt8PtrTy);
 
   llvm::FoldingSetNodeID ID;
   for (Expr *E : Exprs) {
@@ -2539,7 +2540,7 @@
   ".args");
   GV->setSection(AnnotationSection);
   GV->setUnnamedAddr(llvm::GlobalValue::UnnamedAddr::Global);
-  auto *Bitcasted = llvm::ConstantExpr::getBitCast(GV, Int8PtrTy);
+  auto *Bitcasted = llvm::ConstantExpr::getBitCast(GV, GlobalsInt8PtrTy);
 
   Lookup = Bitcasted;
   return Bitcasted;
@@ -2554,17 +2555,19 @@
  *LineNoCst = EmitAnnotationLineNo(L),
  *Args = EmitAnnotationArgs(AA);
 
-  llvm::Constant *ASZeroGV = GV;
-  if (GV->getAddressSpace() != 0) {
-ASZeroGV = llvm::ConstantExpr::getAddrSpaceCast(
-   GV, GV->getValueType()->getPointerTo(0));
+  llvm::Constant *GVInGlobalsAS = GV;
+  if (GV->getAddressSpace() !=
+  getDataLayout().getDefaultGlobalsAddressSpace()) {
+GVInGlobalsAS = llvm::ConstantExpr::getAddrSpaceCast(
+GV, GV->getValueType()->getPointerTo(
+getDataLayout().getDefaultGlobalsAddressSpace()));
   }
 
   // Create the ConstantStruct for the global annotation.
   llvm::Constant *Fields[] = {
-  llvm::ConstantExpr::getBitCast(ASZeroGV, Int8PtrTy),
-  llvm::ConstantExpr::getBitCast(AnnoGV, Int8PtrTy),
-  llvm::ConstantExpr::getBitCast(UnitGV, Int8PtrTy),

[PATCH] D98113: [Driver] Also search FilePaths for compiler-rt before falling back

[Alexander Richardson via Phabricator via cfe-commits]

arichardson added a comment.

In D98113#2872080 , @jroelofs wrote:

>> compiler-rt depends on a libc, typically newlib, which then depends on your 
>> compiler
>
> The builtins should only depend on compiler-provided headers, and not on the 
> rest of libc. Agreed re: the rest of compiler-rt.

As of https://reviews.llvm.org/D103876 that should be true for baremetal 
targets, but clear_cache.c might still need libc/OS headers.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98113

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

[PATCH] D104714: [UpdateCCTestChecks] Support --check-globals

[Alexander Richardson via Phabricator via cfe-commits]

arichardson accepted this revision.
arichardson added a comment.

Thanks for working on this! We have some tests downstream that check globals 
and currently have to use `// UTC_ARGS: --disable` to manually retain them.

The other update script tests compare to an expected output file instead of 
using Filecheck directives. For larger tests the separate files are a lot more 
readable, whereas it doesn't really matter for this test.




Comment at: clang/test/utils/update_cc_test_checks/check-globals.test:34
+RUN: %lit %t
+# Lit was successful.  Sanity-check the results.
+RUN: %lit %t 2>&1 | FileCheck -check-prefix=LIT-RUN %s

Running lit to verify that the output is valid could be something we might want 
to do for the other update script tests. But I guess using the scripts to 
generate tests that are checked it is usually sufficient.



Comment at: llvm/utils/update_cc_test_checks.py:357
   continue  # Don't append the existing CHECK lines
+if line.strip() == '//' + common.SEPARATOR:
+  continue

I feel like this line could do with a comment since it's not immediately 
obvious why it's needed as part of this commit.


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

https://reviews.llvm.org/D104714

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

[PATCH] D100762: [clang][cli] Extract AST dump format into extra option

[Alexander Richardson via Phabricator via cfe-commits]

arichardson added a comment.

In D100762#2707812 , @jansvoboda11 
wrote:

> If `-ast-dump=json` was a driver flag, it would be trivial to pass `-ast-dump 
> -ast-dump-format json` to -cc1 instead. However, aliasing a single option to 
> two options within the -cc1 argument parser isn't possible at the moment 
> AFAIK. I can look into how much work adding that capability would be.
>
> @arichardson Can you point me to the external consumers?

I just did the following search and saw that there are multiple stack overflow 
answers etc. recommending the use of `-Xclang -ast-dump=json`: 
https://www.google.com/search?q=%22-ast-dump%3Djson%22

I am not sure how many actual consumers there are, but I think it would be good 
to keep this option to avoid surprises for users. While they probably shouldn't 
be using internal -cc1 options, this is the only documented way of getting an 
AST dump.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100762

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

[PATCH] D100762: [clang][cli] Extract AST dump format into extra option

[Alexander Richardson via Phabricator via cfe-commits]

arichardson requested changes to this revision.
arichardson added a reviewer: aaron.ballman.
arichardson added a comment.

I'm not sure it's a good idea to remove the `-ast-dump=json` option. While this 
is -cc1 option, there do seem to be external consumers based on a quick search 
for "-ast-dump=json". Keeping it would also reduce some of the test churn.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100762

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

[PATCH] D98286: [Driver] Enable kernel address and memory sanitizers on FreeBSD

[Alexander Richardson 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 rG99eca1bd9c7a: [Driver] Enable kernel address and memory 
sanitizers on FreeBSD (authored by markj, committed by arichardson).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98286

Files:
  clang/lib/Driver/ToolChains/FreeBSD.cpp
  clang/test/Driver/fsanitize.c


Index: clang/test/Driver/fsanitize.c
===
--- clang/test/Driver/fsanitize.c
+++ clang/test/Driver/fsanitize.c
@@ -695,7 +695,13 @@
 // RUN: %clang -target x86_64-unknown-cloudabi -fsanitize=safe-stack %s -### 
2>&1 | FileCheck %s -check-prefix=SAFESTACK-CLOUDABI
 // SAFESTACK-CLOUDABI: "-fsanitize=safe-stack"
 
+// RUN: %clang -target x86_64--freebsd -fsanitize=kernel-address %s -### 2>&1 
| FileCheck %s -check-prefix=KERNEL-ADDRESS-FREEBSD
+// RUN: %clang -target aarch64--freebsd -fsanitize=kernel-address %s -### 2>&1 
| FileCheck %s -check-prefix=KERNEL-ADDRESS-FREEBSD
+// KERNEL-ADDRESS-FREEBSD: "-fsanitize=kernel-address"
 
+// RUN: %clang -target x86_64--freebsd -fsanitize=kernel-memory %s -### 2>&1 | 
FileCheck %s -check-prefix=KERNEL-MEMORY-FREEBSD
+// RUN: %clang -target aarch64--freebsd -fsanitize=kernel-memory %s -### 2>&1 
| FileCheck %s -check-prefix=KERNEL-MEMORY-FREEBSD
+// KERNEL-MEMORY-FREEBSD: "-fsanitize=kernel-memory"
 
 // * NetBSD; please keep ordered as in Sanitizers.def *
 
Index: clang/lib/Driver/ToolChains/FreeBSD.cpp
===
--- clang/lib/Driver/ToolChains/FreeBSD.cpp
+++ clang/lib/Driver/ToolChains/FreeBSD.cpp
@@ -467,6 +467,7 @@
 bool FreeBSD::isPIEDefault() const { return getSanitizerArgs().requiresPIE(); }
 
 SanitizerMask FreeBSD::getSupportedSanitizers() const {
+  const bool IsAArch64 = getTriple().getArch() == llvm::Triple::aarch64;
   const bool IsX86 = getTriple().getArch() == llvm::Triple::x86;
   const bool IsX86_64 = getTriple().getArch() == llvm::Triple::x86_64;
   const bool IsMIPS64 = getTriple().isMIPS64();
@@ -485,8 +486,13 @@
 Res |= SanitizerKind::Fuzzer;
 Res |= SanitizerKind::FuzzerNoLink;
   }
-  if (IsX86_64)
+  if (IsAArch64 || IsX86_64) {
+Res |= SanitizerKind::KernelAddress;
+Res |= SanitizerKind::KernelMemory;
+  }
+  if (IsX86_64) {
 Res |= SanitizerKind::Memory;
+  }
   return Res;
 }
 


Index: clang/test/Driver/fsanitize.c
===
--- clang/test/Driver/fsanitize.c
+++ clang/test/Driver/fsanitize.c
@@ -695,7 +695,13 @@
 // RUN: %clang -target x86_64-unknown-cloudabi -fsanitize=safe-stack %s -### 2>&1 | FileCheck %s -check-prefix=SAFESTACK-CLOUDABI
 // SAFESTACK-CLOUDABI: "-fsanitize=safe-stack"
 
+// RUN: %clang -target x86_64--freebsd -fsanitize=kernel-address %s -### 2>&1 | FileCheck %s -check-prefix=KERNEL-ADDRESS-FREEBSD
+// RUN: %clang -target aarch64--freebsd -fsanitize=kernel-address %s -### 2>&1 | FileCheck %s -check-prefix=KERNEL-ADDRESS-FREEBSD
+// KERNEL-ADDRESS-FREEBSD: "-fsanitize=kernel-address"
 
+// RUN: %clang -target x86_64--freebsd -fsanitize=kernel-memory %s -### 2>&1 | FileCheck %s -check-prefix=KERNEL-MEMORY-FREEBSD
+// RUN: %clang -target aarch64--freebsd -fsanitize=kernel-memory %s -### 2>&1 | FileCheck %s -check-prefix=KERNEL-MEMORY-FREEBSD
+// KERNEL-MEMORY-FREEBSD: "-fsanitize=kernel-memory"
 
 // * NetBSD; please keep ordered as in Sanitizers.def *
 
Index: clang/lib/Driver/ToolChains/FreeBSD.cpp
===
--- clang/lib/Driver/ToolChains/FreeBSD.cpp
+++ clang/lib/Driver/ToolChains/FreeBSD.cpp
@@ -467,6 +467,7 @@
 bool FreeBSD::isPIEDefault() const { return getSanitizerArgs().requiresPIE(); }
 
 SanitizerMask FreeBSD::getSupportedSanitizers() const {
+  const bool IsAArch64 = getTriple().getArch() == llvm::Triple::aarch64;
   const bool IsX86 = getTriple().getArch() == llvm::Triple::x86;
   const bool IsX86_64 = getTriple().getArch() == llvm::Triple::x86_64;
   const bool IsMIPS64 = getTriple().isMIPS64();
@@ -485,8 +486,13 @@
 Res |= SanitizerKind::Fuzzer;
 Res |= SanitizerKind::FuzzerNoLink;
   }
-  if (IsX86_64)
+  if (IsAArch64 || IsX86_64) {
+Res |= SanitizerKind::KernelAddress;
+Res |= SanitizerKind::KernelMemory;
+  }
+  if (IsX86_64) {
 Res |= SanitizerKind::Memory;
+  }
   return Res;
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D55212: Handle alloc_size attribute on function pointers

[Alexander Richardson via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGdc4abca7662b: Handle alloc_size attribute on function 
pointers (authored by arichardson).
Herald added a project: clang.

Changed prior to commit:
  https://reviews.llvm.org/D55212?vs=177519=336519#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D55212

Files:
  clang/include/clang/Basic/Attr.td
  clang/lib/AST/ExprConstant.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/CodeGen/alloc-size-fnptr.c
  clang/test/CodeGen/alloc-size.c
  clang/test/Misc/pragma-attribute-supported-attributes-list.test
  clang/test/Sema/alloc-size.c

Index: clang/test/Sema/alloc-size.c
===
--- clang/test/Sema/alloc-size.c
+++ clang/test/Sema/alloc-size.c
@@ -14,7 +14,7 @@
 
 int fail9(int a) __attribute__((alloc_size(1))); //expected-warning{{'alloc_size' attribute only applies to return values that are pointers}}
 
-int fail10 __attribute__((alloc_size(1))); //expected-warning{{'alloc_size' attribute only applies to functions}}
+int fail10 __attribute__((alloc_size(1))); //expected-warning{{'alloc_size' attribute only applies to non-K functions}}
 
 void *fail11(void *a) __attribute__((alloc_size(1))); //expected-error{{'alloc_size' attribute argument may only refer to a function parameter of integer type}}
 
@@ -22,4 +22,39 @@
 void *fail12(int a) __attribute__((alloc_size(1, "abc"))); //expected-error{{'alloc_size' attribute requires parameter 2 to be an integer constant}}
 void *fail13(int a) __attribute__((alloc_size(1U<<31))); //expected-error{{integer constant expression evaluates to value 2147483648 that cannot be represented in a 32-bit signed integer type}}
 
-int (*PR31453)(int) __attribute__((alloc_size(1))); //expected-warning{{'alloc_size' attribute only applies to functions}}
+void *(*PR31453)(int)__attribute__((alloc_size(1)));
+
+void *KR() __attribute__((alloc_size(1))); //expected-warning{{'alloc_size' attribute only applies to non-K functions}}
+
+// Applying alloc_size to function pointers should work:
+void *(__attribute__((alloc_size(1))) * func_ptr1)(int);
+void *(__attribute__((alloc_size(1, 2))) func_ptr2)(int, int);
+
+// TODO: according to GCC documentation the following should actually be the type
+// “pointer to pointer to alloc_size attributed function returning void*” and should
+// therefore be supported
+void *(__attribute__((alloc_size(1))) **ptr_to_func_ptr)(int); // expected-warning{{'alloc_size' attribute only applies to non-K functions}}
+// The following definitions apply the attribute to the pointer to the function pointer which should not be possible
+void *(*__attribute__((alloc_size(1))) * ptr_to_func_ptr2)(int); // expected-warning{{'alloc_size' attribute only applies to non-K functions}}
+void *(**__attribute__((alloc_size(1))) ptr_to_func_ptr2)(int);  // expected-warning{{'alloc_size' attribute only applies to non-K functions}}
+
+// It should also work for typedefs:
+typedef void *(__attribute__((alloc_size(1))) allocator_function_typdef)(int);
+typedef void *(__attribute__((alloc_size(1, 2))) * allocator_function_typdef2)(int, int);
+void *(__attribute__((alloc_size(1, 2))) * allocator_function_typdef3)(int, int);
+// This typedef applies the alloc_size to the pointer to the function pointer and should not be allowed
+void *(**__attribute__((alloc_size(1, 2))) * allocator_function_typdef4)(int, int); // expected-warning{{'alloc_size' attribute only applies to non-K functions}}
+
+// We should not be warning when assigning function pointers with and without the alloc size attribute
+// since it doesn't change the type of the function
+typedef void *(__attribute__((alloc_size(1))) * my_malloc_fn_pointer_type)(int);
+typedef void *(*my_other_malloc_fn_pointer_type)(int);
+void *fn(int i);
+__attribute__((alloc_size(1))) void *fn2(int i);
+
+int main() {
+  my_malloc_fn_pointer_type f = fn;
+  my_other_malloc_fn_pointer_type f2 = fn;
+  my_malloc_fn_pointer_type f3 = fn2;
+  my_other_malloc_fn_pointer_type f4 = fn2;
+}
Index: clang/test/Misc/pragma-attribute-supported-attributes-list.test
===
--- clang/test/Misc/pragma-attribute-supported-attributes-list.test
+++ clang/test/Misc/pragma-attribute-supported-attributes-list.test
@@ -12,7 +12,6 @@
 // CHECK-NEXT: AcquireHandle (SubjectMatchRule_function, SubjectMatchRule_type_alias, SubjectMatchRule_variable_is_parameter)
 // CHECK-NEXT: Alias (SubjectMatchRule_function, SubjectMatchRule_variable_is_global)
 // CHECK-NEXT: AlignValue (SubjectMatchRule_variable, SubjectMatchRule_type_alias)
-// CHECK-NEXT: AllocSize (SubjectMatchRule_function)
 // CHECK-NEXT: AlwaysDestroy (SubjectMatchRule_variable)
 // CHECK-NEXT: AlwaysInline (SubjectMatchRule_function)
 // CHECK-NEXT: Annotate ()
Index: 

[PATCH] D100054: Handle flags such as -m32 when computing the prefix for programs/runtime libs

[Alexander Richardson via Phabricator via cfe-commits]

arichardson updated this revision to Diff 336483.
arichardson added a comment.

- Fix Windows path regex


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100054

Files:
  clang/include/clang/Driver/Driver.h
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChain.cpp
  clang/lib/Driver/ToolChains/RISCVToolchain.cpp
  
clang/test/Driver/Inputs/basic_freebsd64_tree/usr/bin/i386-unknown-freebsd12.2-ld
  clang/test/Driver/Inputs/basic_freebsd64_tree/usr/bin/x86_64-freebsd12.2-ld
  
clang/test/Driver/Inputs/basic_freebsd64_tree/usr/bin/x86_64-unknown-freebsd12.2-ld
  clang/test/Driver/freebsd-m32.c
  clang/test/Driver/linux-per-target-runtime-dir.c

Index: clang/test/Driver/linux-per-target-runtime-dir.c
===
--- clang/test/Driver/linux-per-target-runtime-dir.c
+++ clang/test/Driver/linux-per-target-runtime-dir.c
@@ -25,3 +25,18 @@
 // RUN: -resource-dir=%S/Inputs/resource_dir_with_per_target_subdir \
 // RUN:   | FileCheck --check-prefix=CHECK-FILE-NAME-X8664 %s
 // CHECK-FILE-NAME-X8664: lib{{/|\\}}x86_64-linux-gnu{{/|\\}}libclang_rt.builtins.a
+
+/// Check that we handle flags such as -m32 when searching for the builtins:
+/// Previously clang would use the raw triple passed to -target to find builtins
+/// and sanitizer libraries, but this will result in build errors when compiling
+/// with flags such as -m32. Check that we use the adjusted triple instead:
+// RUN: %clang -rtlib=compiler-rt -print-file-name=libclang_rt.builtins.a 2>&1 \
+// RUN: --target=i386-linux-gnu \
+// RUN: -resource-dir=%S/Inputs/resource_dir_with_per_target_subdir \
+// RUN:   | FileCheck --check-prefix=CHECK-FILE-NAME-I386 %s
+// CHECK-FILE-NAME-I386: resource_dir_with_per_target_subdir{{/|\\}}lib{{/|\\}}i386-linux-gnu{{/|\\}}libclang_rt.builtins.a
+// RUN: %clang -rtlib=compiler-rt -print-file-name=libclang_rt.builtins.a 2>&1 \
+// RUN: --target=x86_64-linux-gnu -m32 \
+// RUN: -resource-dir=%S/Inputs/resource_dir_with_per_target_subdir \
+// RUN:   | FileCheck --check-prefix=CHECK-FILE-NAME-X8664-M32 %s
+// CHECK-FILE-NAME-X8664-M32: resource_dir_with_per_target_subdir{{/|\\}}lib{{/|\\}}i386-linux-gnu{{/|\\}}libclang_rt.builtins.a
Index: clang/test/Driver/freebsd-m32.c
===
--- /dev/null
+++ clang/test/Driver/freebsd-m32.c
@@ -0,0 +1,76 @@
+/// Check that building with -m32 links with i386-freebsd12.2-ld/ instead of
+/// x86_64-freebsd12.2-ld and that we select the right sanitizer runtime.
+
+/// We should select x86_64-unknown-freebsd12.2-ld since it matches the triple argument
+/// Note: The paths specified by -B are not searched for triple-prefixed tools, so
+/// we also have to set $PATH.
+// RUN: env PATH=%S/Inputs/basic_freebsd64_tree/usr/bin %clang -no-canonical-prefixes \
+// RUN:   -target x86_64-unknown-freebsd12.2 %s \
+// RUN:   -B%S/Inputs/basic_freebsd64_tree/usr/bin -### 2>&1 | FileCheck %s --check-prefix=PREFIXED-64
+// PREFIXED-64: "-cc1" "-triple" "x86_64-unknown-freebsd12.2"
+// PREFIXED-64-NEXT: "{{.+}}Inputs{{/|}}basic_freebsd64_tree{{/|}}usr{{/|}}bin{{/|}}x86_64-unknown-freebsd12.2-ld" "--eh-frame-hdr"
+// Should not be passing an explicit linker emulation for the 64-bit case
+// PREFIXED-64-NOT: "-m"
+// RUN: env PATH=/this/path/does/not/exist %clang -no-canonical-prefixes \
+// RUN:   -target x86_64-unknown-freebsd12.2 %s \
+// RUN:   -B%S/Inputs/basic_freebsd64_tree/usr/bin -### 2>&1 | FileCheck %s --check-prefix=MINUS-B-NO-TRIPLE-PREFIX
+// MINUS-B-NO-TRIPLE-PREFIX: "-cc1" "-triple" "x86_64-unknown-freebsd12.2"
+// MINUS-B-NO-TRIPLE-PREFIX-NEXT: "ld" "--eh-frame-hdr"
+// MINUS-B-NO-TRIPLE-PREFIX-NOT: "-m"
+
+/// The triple passed to clang -cc1 should be normalized, but the prefix when searching
+/// for ld should not be normalized. Since there is no x86_64--freebsd12.2-ld, passing
+/// -target x86_64--freebsd12.2 should not find a a valid linker:
+// RUN: env PATH=%S/Inputs/basic_freebsd64_tree/usr/bin %clang -no-canonical-prefixes \
+// RUN:   -target x86_64--freebsd12.2 %s \
+// RUN:   -B%S/Inputs/basic_freebsd64_tree/usr/bin -### 2>&1 | FileCheck %s --check-prefix=NO-NORMALIZE-LD-PREFIX-64
+// NO-NORMALIZE-LD-PREFIX-64: "-cc1" "-triple" "x86_64-unknown-freebsd12.2"
+// NO-NORMALIZE-LD-PREFIX-64-NEXT: "ld" "--eh-frame-hdr"
+// NO-NORMALIZE-LD-PREFIX-NOT: "-m"
+
+/// We should search for i386-unknown-freebsd12.2-ld when -m32 is passed (and also pass -m elf_i386_fbsd):
+// RUN: env PATH=%S/Inputs/basic_freebsd64_tree/usr/bin %clang -no-canonical-prefixes \
+// RUN:   -target x86_64-unknown-freebsd12.2 %s -m32 \
+// RUN:   -B%S/Inputs/basic_freebsd64_tree/usr/bin -### 2>&1  | FileCheck %s --check-prefix=PREFIXED-M32
+// PREFIXED-M32: "-cc1" "-triple" "i386-unknown-freebsd12.2"
+// PREFIXED-M32-NEXT: