[PATCH] D69764: [clang-format] Add East/West Const fixer capability

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

If this is something we think we wouldn't want in clang-format, what about the 
idea of building a new binary which did allow the modification of the tokens, 
This comes up from time to time and I kind of feel its a weak argument for 
those that don't want to use it enforcing their will on those that do.

I feel we have D69764: [clang-format] Add East/West Const fixer capability 
 and D95168: [clang-format] Add InsertBraces 
option  both of which seem to break the 
fundamental belief that some feel that we shouldn't change anything other than 
whitespace (A fact that has already been broken 3 times already).

But really these are just separate passes, so why couldn't clang-format allow 
custom passes to be added much like any compiler does.

If the objection is that this isn't clang-format's role then what if we build 
something new whose purpose IS to allow these kinds of changes?

I'm pretty rubbish with names but what about:

  clang-format++
  clang-modifier
  clang-rewriter

I think it still has some value to those that want to use it, and I'd like to 
find an elegant way to deliver it.

  if (Style.Language == FormatStyle::LK_Cpp) {
if (Style.FixNamespaceComments)
  Passes.emplace_back([&](const Environment &Env) {
return NamespaceEndCommentsFixer(Env, Expanded).process();
  });
  
if (Style.SortUsingDeclarations)
  Passes.emplace_back([&](const Environment &Env) {
return UsingDeclarationsSorter(Env, Expanded).process();
  });
  
if (Style.InsertBraces != FormatStyle::BIS_Never)
  Passes.emplace_back([&](const Environment &Env) {
return BracesInserter(Env, Expanded).process();
  });
  
if (Style.isCpp() && Style.ConstPlacement != FormatStyle::CS_Leave)
Passes.emplace_back([&](const Environment &Env) {
  return EastWestConstFixer(Env, Expanded).process();
});
  }
  
  if (Style.Language == FormatStyle::LK_JavaScript &&
  Style.JavaScriptQuotes != FormatStyle::JSQS_Leave)
Passes.emplace_back([&](const Environment &Env) {
  return JavaScriptRequoter(Env, Expanded).process();
});
  
  Passes.emplace_back([&](const Environment &Env) {
return Formatter(Env, Expanded, Status).process();
  });
  
  if (Style.Language == FormatStyle::LK_JavaScript &&
  Style.InsertTrailingCommas == FormatStyle::TCS_Wrapped)
Passes.emplace_back([&](const Environment &Env) {
  return TrailingCommaInserter(Env, Expanded).process();
});


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

https://reviews.llvm.org/D69764

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

[PATCH] D105457: [clang] Refactor AST printing tests to share more infrastructure

[Douglas Yung via Phabricator via cfe-commits]

dyung added a comment.

This change I suspect is causing a build failure on a build bot.

https://lab.llvm.org/buildbot/#/builders/110/builds/4827

  FAILED: /usr/bin/c++  -DCLANG_ROUND_TRIP_CC1_ARGS=ON -DGTEST_HAS_RTTI=0 
-D_DEBUG -D_GNU_SOURCE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS 
-D__STDC_LIMIT_MACROS -Itools/clang/unittests/AST 
-I/home/ssglocal/clang-cmake-x86_64-avx2-linux/clang-cmake-x86_64-avx2-linux/llvm/clang/unittests/AST
 
-I/home/ssglocal/clang-cmake-x86_64-avx2-linux/clang-cmake-x86_64-avx2-linux/llvm/clang/include
 -Itools/clang/include -Iinclude 
-I/home/ssglocal/clang-cmake-x86_64-avx2-linux/clang-cmake-x86_64-avx2-linux/llvm/llvm/include
 
-I/home/ssglocal/clang-cmake-x86_64-avx2-linux/clang-cmake-x86_64-avx2-linux/llvm/llvm/utils/unittest/googletest/include
 
-I/home/ssglocal/clang-cmake-x86_64-avx2-linux/clang-cmake-x86_64-avx2-linux/llvm/llvm/utils/unittest/googlemock/include
 -march=broadwell -fPIC -fno-semantic-interposition -fvisibility-inlines-hidden 
-Werror=date-time -Wall -Wextra -Wno-unused-parameter -Wwrite-strings 
-Wcast-qual -Wno-missing-field-initializers -pedantic -Wno-long-long 
-Wno-maybe-uninitialized -Wdelete-non-virtual-dtor -Wno-comment 
-fdiagnostics-color -ffunction-sections -fdata-sections -fno-common 
-Woverloaded-virtual -fno-strict-aliasing -O3 -DNDEBUG-Wno-variadic-macros 
-fno-exceptions -fno-rtti -UNDEBUG -Wno-suggest-override -std=c++14 -MD -MT 
tools/clang/unittests/AST/CMakeFiles/ASTTests.dir/StmtPrinterTest.cpp.o -MF 
tools/clang/unittests/AST/CMakeFiles/ASTTests.dir/StmtPrinterTest.cpp.o.d -o 
tools/clang/unittests/AST/CMakeFiles/ASTTests.dir/StmtPrinterTest.cpp.o -c 
/home/ssglocal/clang-cmake-x86_64-avx2-linux/clang-cmake-x86_64-avx2-linux/llvm/clang/unittests/AST/StmtPrinterTest.cpp
  /tmp/cc8Ycpsn.s: Assembler messages:
  /tmp/cc8Ycpsn.s:11414: Error: symbol 
`_ZNSt17_Function_handlerIFbPKN5clang4StmtEENS0_UlS3_E2_EE9_M_invokeERKSt9_Any_dataOS3_'
 is already defined
  /tmp/cc8Ycpsn.s:11937: Error: symbol 
`_ZNSt14_Function_base13_Base_managerIN5clangUlPKNS1_4StmtEE2_EE10_M_managerERSt9_Any_dataRKS7_St18_Manager_operation'
 is already defined
  ninja: build stopped: subcommand failed.

Can you take a look?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105457

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

[PATCH] D105876: OMPIRBuilder for Interop directive

[Sri Hari Krishna Narayanan via Phabricator via cfe-commits]

sriharikrishna created this revision.
sriharikrishna added reviewers: jdoerfert, ABataev, RaviNarayanaswamy.
Herald added a subscriber: hiraditya.
sriharikrishna requested review of this revision.
Herald added subscribers: llvm-commits, cfe-commits, sstefan1.
Herald added projects: clang, LLVM.

Implements the OMPIRBuilder portion for the
Interop directive.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D105876

Files:
  clang/lib/CodeGen/CGStmt.cpp
  clang/lib/CodeGen/CGStmtOpenMP.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/test/OpenMP/interop_irbuilder.cpp
  llvm/include/llvm/Frontend/OpenMP/OMPConstants.h
  llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
  llvm/include/llvm/Frontend/OpenMP/OMPKinds.def
  llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp

Index: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
===
--- llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
+++ llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
@@ -2173,6 +2173,96 @@
   return Builder.CreateCall(Fn, Args, Name);
 }
 
+CallInst *OpenMPIRBuilder::createOMPInteropInit(const LocationDescription &Loc,
+Value *InteropVar,
+OMPInteropType InteropType,
+llvm::Value *Device,
+llvm::Value *NumDependences,
+llvm::Value *DependenceAddress,
+int HaveNowaitClause) {
+  IRBuilder<>::InsertPointGuard IPG(Builder);
+  Builder.restoreIP(Loc.IP);
+
+  Constant *SrcLocStr = getOrCreateSrcLocStr(Loc);
+  Value *Ident = getOrCreateIdent(SrcLocStr);
+  Value *ThreadId = getOrCreateThreadID(Ident);
+  if (Device == NULL)
+Device = ConstantInt::get(M.getContext(), APInt(32, -1, true));
+  ConstantInt *InteropTypeVal =
+  ConstantInt::get(M.getContext(), APInt(64, (int)InteropType, true));
+  if (NumDependences == nullptr) {
+NumDependences = ConstantInt::get(M.getContext(), APInt(32, 0, true));
+PointerType *PointerTy_0 = llvm::Type::getInt8PtrTy(M.getContext());
+DependenceAddress = ConstantPointerNull::get(PointerTy_0);
+  }
+  Value *HaveNowaitClauseVal =
+  ConstantInt::get(M.getContext(), APInt(32, HaveNowaitClause, true));
+  Value *Args[] = {
+  Ident,  ThreadId,   InteropVar,InteropTypeVal,
+  Device, NumDependences, DependenceAddress, HaveNowaitClauseVal};
+
+  Function *Fn = getOrCreateRuntimeFunctionPtr(OMPRTL___kmpc_interop_init);
+
+  return Builder.CreateCall(Fn, Args);
+}
+
+CallInst *OpenMPIRBuilder::createOMPInteropDestroy(
+const LocationDescription &Loc, Value *InteropVar, llvm::Value *Device,
+llvm::Value *NumDependences, llvm::Value *DependenceAddress,
+int HaveNowaitClause) {
+  IRBuilder<>::InsertPointGuard IPG(Builder);
+  Builder.restoreIP(Loc.IP);
+
+  Constant *SrcLocStr = getOrCreateSrcLocStr(Loc);
+  Value *Ident = getOrCreateIdent(SrcLocStr);
+  Value *ThreadId = getOrCreateThreadID(Ident);
+  if (Device == NULL)
+Device = ConstantInt::get(M.getContext(), APInt(32, -1, true));
+  if (NumDependences == nullptr) {
+NumDependences = ConstantInt::get(M.getContext(), APInt(32, 0, true));
+PointerType *PointerTy_0 = llvm::Type::getInt8PtrTy(M.getContext());
+DependenceAddress = ConstantPointerNull::get(PointerTy_0);
+  }
+  Value *HaveNowaitClauseVal =
+  ConstantInt::get(M.getContext(), APInt(32, HaveNowaitClause, true));
+  Value *Args[] = {
+  Ident,  ThreadId,  InteropVar, Device,
+  NumDependences, DependenceAddress, HaveNowaitClauseVal};
+
+  Function *Fn = getOrCreateRuntimeFunctionPtr(OMPRTL___kmpc_interop_destroy);
+
+  return Builder.CreateCall(Fn, Args);
+}
+
+CallInst *OpenMPIRBuilder::createOMPInteropUse(const LocationDescription &Loc,
+   Value *InteropVar,
+   llvm::Value *Device,
+   llvm::Value *NumDependences,
+   llvm::Value *DependenceAddress,
+   int HaveNowaitClause) {
+  IRBuilder<>::InsertPointGuard IPG(Builder);
+  Builder.restoreIP(Loc.IP);
+  Constant *SrcLocStr = getOrCreateSrcLocStr(Loc);
+  Value *Ident = getOrCreateIdent(SrcLocStr);
+  Value *ThreadId = getOrCreateThreadID(Ident);
+  if (Device == NULL)
+Device = ConstantInt::get(M.getContext(), APInt(32, -1, true));
+  if (NumDependences == nullptr) {
+NumDependences = ConstantInt::get(M.getContext(), APInt(32, 0, true));
+PointerType *PointerTy_0 = llvm::Type::getInt8PtrTy(M.getContext());
+DependenceAddress = ConstantPointerNull::get(PointerTy_0);
+  }
+  Value *HaveNowaitClauseVal =
+  ConstantInt::get(M.getContext(), APInt(32, HaveNowaitClause, true));
+  Value *

[PATCH] D105168: [RISCV] Unify the arch string parsing logic to RISCVISAInfo.

[Kito Cheng via Phabricator via cfe-commits]

kito-cheng updated this revision to Diff 358193.
kito-cheng added a comment.

Changes:

- Handle arch attribute emition in RISCVTargetStreamer.cpp, thanks @khchen for 
reminding me that!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105168

Files:
  clang/lib/Basic/Targets/RISCV.cpp
  clang/lib/Basic/Targets/RISCV.h
  clang/lib/Driver/ToolChains/Arch/RISCV.cpp
  clang/test/Driver/riscv-abi.c
  clang/test/Driver/riscv-arch.c
  llvm/include/llvm/Support/RISCVISAInfo.h
  llvm/lib/Support/CMakeLists.txt
  llvm/lib/Support/RISCVISAInfo.cpp
  llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp
  llvm/lib/Target/RISCV/MCTargetDesc/RISCVBaseInfo.cpp
  llvm/lib/Target/RISCV/MCTargetDesc/RISCVBaseInfo.h
  llvm/lib/Target/RISCV/MCTargetDesc/RISCVTargetStreamer.cpp
  llvm/test/MC/RISCV/attribute-arch.s
  llvm/test/MC/RISCV/attribute-with-insts.s
  llvm/test/MC/RISCV/invalid-attribute.s

Index: llvm/test/MC/RISCV/invalid-attribute.s
===
--- llvm/test/MC/RISCV/invalid-attribute.s
+++ llvm/test/MC/RISCV/invalid-attribute.s
@@ -7,10 +7,10 @@
 # RUN: not llvm-mc %s -triple=riscv64 -filetype=asm 2>&1 | FileCheck %s
 
 .attribute arch, "foo"
-# CHECK: [[@LINE-1]]:18: error: bad arch string foo
+# CHECK: [[@LINE-1]]:18: error: invalid arch name 'foo', string must begin with rv32{i,e,g} or rv64{i,g}
 
 .attribute arch, "rv32i2p0_y2p0"
-# CHECK: [[@LINE-1]]:18: error: bad arch string y2p0
+# CHECK: [[@LINE-1]]:18: error: invalid arch name 'rv32i2p0_y2p0', invalid standard user-level extension 'y'
 
 .attribute stack_align, "16"
 # CHECK: [[@LINE-1]]:25: error: expected numeric constant
Index: llvm/test/MC/RISCV/attribute-with-insts.s
===
--- llvm/test/MC/RISCV/attribute-with-insts.s
+++ llvm/test/MC/RISCV/attribute-with-insts.s
@@ -10,7 +10,7 @@
 # RUN:   | llvm-objdump --triple=riscv64 -d -M no-aliases - \
 # RUN:   | FileCheck -check-prefix=CHECK-INST %s
 
-.attribute arch, "rv64i2p0_m2p0_a2p0_d2p0_c2p0"
+.attribute arch, "rv64i2p0_m2p0_a2p0_f2p0_d2p0_c2p0"
 
 # CHECK-INST: lr.w t0, (t1)
 lr.w t0, (t1)
Index: llvm/test/MC/RISCV/attribute-arch.s
===
--- llvm/test/MC/RISCV/attribute-arch.s
+++ llvm/test/MC/RISCV/attribute-arch.s
@@ -9,9 +9,6 @@
 .attribute arch, "rv32i2"
 # CHECK: attribute  5, "rv32i2p0"
 
-.attribute arch, "rv32i2p"
-# CHECK: attribute  5, "rv32i2p0"
-
 .attribute arch, "rv32i2p0"
 # CHECK: attribute  5, "rv32i2p0"
 
@@ -33,14 +30,14 @@
 .attribute arch, "rv32ima2p0_fdc"
 # CHECK: attribute  5, "rv32i2p0_m2p0_a2p0_f2p0_d2p0_c2p0"
 
-.attribute arch, "rv32ima2p_fdc"
+.attribute arch, "rv32ima2p0_fdc"
 # CHECK: attribute  5, "rv32i2p0_m2p0_a2p0_f2p0_d2p0_c2p0"
 
 .attribute arch, "rv32ib"
 # CHECK: attribute  5, "rv32i2p0_b0p93_zba0p93_zbb0p93_zbc0p93_zbe0p93_zbf0p93_zbm0p93_zbp0p93_zbr0p93_zbs0p93_zbt0p93"
 
 .attribute arch, "rv32iv"
-# CHECK: attribute  5, "rv32i2p0_v0p10"
+# CHECK: attribute  5, "rv32i2p0_v0p10_zvlsseg0p10"
 
 .attribute arch, "rv32izba"
 # CHECK: attribute  5, "rv32i2p0_zba0p93"
Index: llvm/lib/Target/RISCV/MCTargetDesc/RISCVTargetStreamer.cpp
===
--- llvm/lib/Target/RISCV/MCTargetDesc/RISCVTargetStreamer.cpp
+++ llvm/lib/Target/RISCV/MCTargetDesc/RISCVTargetStreamer.cpp
@@ -11,9 +11,11 @@
 //===--===//
 
 #include "RISCVTargetStreamer.h"
+#include "RISCVBaseInfo.h"
 #include "RISCVMCTargetDesc.h"
 #include "llvm/Support/FormattedStream.h"
 #include "llvm/Support/RISCVAttributes.h"
+#include "llvm/Support/RISCVISAInfo.h"
 
 using namespace llvm;
 
@@ -43,57 +45,14 @@
   else
 emitAttribute(RISCVAttrs::STACK_ALIGN, RISCVAttrs::ALIGN_16);
 
-  std::string Arch = "rv32";
-  if (STI.hasFeature(RISCV::Feature64Bit))
-Arch = "rv64";
-  if (STI.hasFeature(RISCV::FeatureRV32E))
-Arch += "e1p9";
-  else
-Arch += "i2p0";
-  if (STI.hasFeature(RISCV::FeatureStdExtM))
-Arch += "_m2p0";
-  if (STI.hasFeature(RISCV::FeatureStdExtA))
-Arch += "_a2p0";
-  if (STI.hasFeature(RISCV::FeatureStdExtF))
-Arch += "_f2p0";
-  if (STI.hasFeature(RISCV::FeatureStdExtD))
-Arch += "_d2p0";
-  if (STI.hasFeature(RISCV::FeatureStdExtC))
-Arch += "_c2p0";
-  if (STI.hasFeature(RISCV::FeatureStdExtB))
-Arch += "_b0p93";
-  if (STI.hasFeature(RISCV::FeatureStdExtV))
-Arch += "_v0p10";
-  if (STI.hasFeature(RISCV::FeatureExtZfh))
-Arch += "_zfh0p1";
-  if (STI.hasFeature(RISCV::FeatureExtZba))
-Arch += "_zba0p93";
-  if (STI.hasFeature(RISCV::FeatureExtZbb))
-Arch += "_zbb0p93";
-  if (STI.hasFeature(RISCV::FeatureExtZbc))
-Arch += "_zbc0p93";
-  if (STI.hasFeature(RISCV::FeatureExtZbe))
-Arch += "_zbe0p93";
-  if (STI.ha

[PATCH] D105629: [TSan] Add SystemZ support

[Ulrich Weigand via Phabricator via cfe-commits]

uweigand added a comment.

See inline comments ... otherwise the SystemZ platform-specific parts look good 
to me.




Comment at: compiler-rt/lib/tsan/rtl/tsan_platform_posix.cpp:123
+#if defined(__s390x__)
+  ProtectRange(HiAppMemEnd(), 0xf000ull);
+#endif

Did you test this on older kernels without 5-level page table support?   I 
believe the allocation / mprotect may fail on those ...



Comment at: compiler-rt/lib/tsan/rtl/tsan_rtl_s390x.S:22
+  CFI_REL_OFFSET(%r2, R2_REL_OFFSET)
+  CFI_REL_OFFSET(%r3, R3_REL_OFFSET)
+  stmg %r14, %r15, R14_REL_OFFSET(%r15)

Do we need CFI for r2/r3 ?  Those are call-clobbered any cannot be unwound 
normally anyway ...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105629

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

[PATCH] D105384: [NVPTX, CUDA] Add .and.popc variant of the b1 MMA instruction.

[Steffen Larsen via Phabricator via cfe-commits]

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

Thank you for addressing my concerns. I am happy with the changes. Great work!




Comment at: clang/test/CodeGen/builtins-nvptx-mma.py:35-38
+def make_mma_ops(geoms, types_a, types_b, types_c, types_d, b1ops=None):
   ops = []
+  if b1ops is None:
+b1ops = [""]

tra wrote:
> steffenlarsen wrote:
> > 
> Default initializers that use objects in python are one of the common gotchas.
> https://docs.python-guide.org/writing/gotchas/#mutable-default-arguments
> 
> It probably does not make much of a difference in this case as we do not 
> modify it, but I'd prefer to avoid it nevertheless.
Interesting and a little horrifying. I agree it wouldn't have been a problem in 
this particular case, but I understand your concern and I am fine with keeping 
it as-is. 👍 



Comment at: clang/test/CodeGen/builtins-nvptx-mma.py:84
+  # It uses __mma_tf32_m16n16k8_ld_c but __mma_m16n16k8_st_c_f32.
+  make_ldst_ops(["m16n16k8"], ["a", "b", "c", "d"], ["tf32", "f32"]))
 

tra wrote:
> steffenlarsen wrote:
> > The following changes would remove the need for the `m16n16k8` cases in 
> > `is_ldst_variant_supported`.
> This was done deliberately. I did have that in an earlier variant of the 
> patch, but eventually settled on the current version.
> 
> My thinking is that `make_ldst_ops(m16n16k8)` would create whatever is 
> necessary for `m16n16k8`, and we'd keep the decision of what to produce in 
> one place in `is_ldst_variant_supported`. Splitting one make_ldst_ops into 
> two here makes this decision implicit which would need additional 
> explanations. Code that needs less explanations  wins. :-)
> 
My concern is that it is more confusing this way because they are the only 
load/store ops generated where the type is then later filtered. It makes sense 
for MMA because it needs to filter out select combinations, but here the 
comment above says one thing and `make_ldst_ops` does something else.

I don't think it makes a huge difference either way, so if you prefer the 
current version I am okay with keeping it. That said, it might be good to have 
a comment in the `m16n16k8` case of `is_ldst_variant_supported` making the 
connection to this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105384

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

[PATCH] D105690: [RISCV] Rename assembler mnemonic of unordered floating-point reductions for v1.0-rc change

[Fraser Cormack via Phabricator via cfe-commits]

frasercrmck added inline comments.



Comment at: llvm/test/CodeGen/RISCV/rvv/vfredusum-rv32.ll:1
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc -mtriple=riscv32 -mattr=+experimental-v,+d,+experimental-zfh 
-verify-machineinstrs \

Just to check - was this renaming done with `git mv`? Phabricator suggests that 
`vfredsum-rv32.ll` was deleted and this was added, which would be worse for the 
git history. It might be a phabricator quirk, though.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105690

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

[PATCH] D105421: [analyzer] Handle << operator for std::unique_ptr

[Deep Majumder via Phabricator via cfe-commits]

RedDocMD updated this revision to Diff 358204.
RedDocMD added a comment.

Removed stupid mistakes


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105421

Files:
  clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp
  clang/test/Analysis/Inputs/system-header-simulator-cxx.h
  clang/test/Analysis/smart-ptr.cpp

Index: clang/test/Analysis/smart-ptr.cpp
===
--- clang/test/Analysis/smart-ptr.cpp
+++ clang/test/Analysis/smart-ptr.cpp
@@ -3,6 +3,11 @@
 // RUN:   -analyzer-config cplusplus.SmartPtrModeling:ModelSmartPtrDereference=true\
 // RUN:   -std=c++11 -verify %s
 
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection\
+// RUN:   -analyzer-checker cplusplus.Move,alpha.cplusplus.SmartPtr\
+// RUN:   -analyzer-config cplusplus.SmartPtrModeling:ModelSmartPtrDereference=true\
+// RUN:   -std=c++20 -verify %s
+
 #include "Inputs/system-header-simulator-cxx.h"
 
 void clang_analyzer_warnIfReached();
@@ -457,3 +462,28 @@
 P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
   }
 }
+
+#if __cplusplus >= 202002L
+
+void testOstreamOverload(std::unique_ptr P) {
+  auto &Cout = std::cout;
+  auto &PtrCout = std::cout << P;
+  auto &StringCout = std::cout << "hello";
+  // We are testing the fact that in our modelling of
+  // operator<<(basic_ostream &, const unique_ptr &)
+  // we set the return SVal to the SVal of the ostream arg.
+  clang_analyzer_eval(&Cout == &PtrCout);// expected-warning {{TRUE}}
+  // FIXME: Technically, they should be equal,
+  // that hasn't been modelled yet.
+  clang_analyzer_eval(&Cout == &StringCout); // expected-warning {{UNKNOWN}}
+}
+
+int glob;
+void testOstreamDoesntInvalidateGlobals(std::unique_ptr P) {
+  int x = glob;
+  std::cout << P;
+  int y = glob;
+  clang_analyzer_eval(x == y); // expected-warning {{TRUE}}
+}
+
+#endif
Index: clang/test/Analysis/Inputs/system-header-simulator-cxx.h
===
--- clang/test/Analysis/Inputs/system-header-simulator-cxx.h
+++ clang/test/Analysis/Inputs/system-header-simulator-cxx.h
@@ -981,6 +981,22 @@
 } // namespace std
 #endif
 
+namespace std {
+template 
+class basic_ostream;
+
+using ostream = basic_ostream;
+
+extern std::ostream cout;
+
+ostream &operator<<(ostream &, const string &);
+
+#if __cplusplus >= 202002L
+template 
+ostream &operator<<(ostream &, const std::unique_ptr &);
+#endif
+} // namespace std
+
 #ifdef TEST_INLINABLE_ALLOCATORS
 namespace std {
   void *malloc(size_t);
Index: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp
+++ clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp
@@ -68,6 +68,7 @@
   bool updateMovedSmartPointers(CheckerContext &C, const MemRegion *ThisRegion,
 const MemRegion *OtherSmartPtrRegion) const;
   void handleBoolConversion(const CallEvent &Call, CheckerContext &C) const;
+  bool handleOstreamOperator(const CallEvent &Call, CheckerContext &C) const;
 
   using SmartPtrMethodHandlerFn =
   void (SmartPtrModeling::*)(const CallEvent &Call, CheckerContext &) const;
@@ -81,6 +82,31 @@
 
 REGISTER_MAP_WITH_PROGRAMSTATE(TrackedRegionMap, const MemRegion *, SVal)
 
+// Checks if RD has name in Names and is in std namespace
+static bool hasStdClassWithName(const CXXRecordDecl *RD,
+ArrayRef Names) {
+  if (!RD || !RD->getDeclContext()->isStdNamespace())
+return false;
+  if (RD->getDeclName().isIdentifier()) {
+StringRef Name = RD->getName();
+return llvm::any_of(Names, [&Name](StringRef GivenName) -> bool {
+  return Name == GivenName;
+});
+  }
+  return false;
+}
+
+constexpr llvm::StringLiteral STD_PTR_NAMES[] = {"shared_ptr", "unique_ptr",
+ "weak_ptr"};
+
+static bool isStdSmartPtr(const CXXRecordDecl *RD) {
+  return hasStdClassWithName(RD, STD_PTR_NAMES);
+}
+
+static bool isStdSmartPtr(const Expr *E) {
+  return isStdSmartPtr(E->getType()->getAsCXXRecordDecl());
+}
+
 // Define the inter-checker API.
 namespace clang {
 namespace ento {
@@ -89,16 +115,7 @@
   const auto *MethodDecl = dyn_cast_or_null(Call.getDecl());
   if (!MethodDecl || !MethodDecl->getParent())
 return false;
-
-  const auto *RecordDecl = MethodDecl->getParent();
-  if (!RecordDecl || !RecordDecl->getDeclContext()->isStdNamespace())
-return false;
-
-  if (RecordDecl->getDeclName().isIdentifier()) {
-StringRef Name = RecordDecl->getName();
-return Name == "shared_ptr" || Name == "unique_ptr" || Name == "weak_ptr";
-  }
-  return false;
+  return isStdSmartPtr(MethodDecl->getParent());
 }
 
 bool isNullSmartPtr(const ProgramStateRef State, const MemRegion *ThisRegi

[PATCH] D105092: [PoC][RISCV] Add the tail policy argument to builtins/intrinsics.

[Hsiangkai Wang via Phabricator via cfe-commits]

HsiangKai updated this revision to Diff 358205.
HsiangKai added a comment.

Add the TA argument to most of the intrinsics with mask.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105092

Files:
  clang/include/clang/Basic/riscv_vector.td
  clang/test/CodeGen/RISCV/rvv-intrinsics/vadd-policy.c
  clang/utils/TableGen/RISCVVEmitter.cpp
  llvm/include/llvm/IR/IntrinsicsRISCV.td
  llvm/lib/Target/RISCV/MCTargetDesc/RISCVBaseInfo.h
  llvm/lib/Target/RISCV/RISCVInsertVSETVLI.cpp
  llvm/lib/Target/RISCV/RISCVInstrFormats.td
  llvm/lib/Target/RISCV/RISCVInstrInfoVPseudos.td
  llvm/lib/Target/RISCV/RISCVInstrInfoVVLPatterns.td
  llvm/test/CodeGen/RISCV/rvv/vadd-policy.ll

Index: llvm/test/CodeGen/RISCV/rvv/vadd-policy.ll
===
--- /dev/null
+++ llvm/test/CodeGen/RISCV/rvv/vadd-policy.ll
@@ -0,0 +1,65 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc -mtriple=riscv64 -mattr=+experimental-v -verify-machineinstrs \
+; RUN:   --riscv-no-aliases < %s | FileCheck %s
+
+declare  @llvm.riscv.vadd.nxv8i8.nxv8i8(
+  ,
+  ,
+  i64);
+
+define  @intrinsic_vadd_vv_nxv8i8_nxv8i8_nxv8i8( %0,  %1, i64 %2) nounwind {
+; CHECK-LABEL: intrinsic_vadd_vv_nxv8i8_nxv8i8_nxv8i8:
+; CHECK:   # %bb.0: # %entry
+; CHECK-NEXT:vsetvli zero, a0, e8, m1, ta, mu
+; CHECK-NEXT:vadd.vv v8, v8, v9
+; CHECK-NEXT:jalr zero, 0(ra)
+entry:
+  %a = call  @llvm.riscv.vadd.nxv8i8.nxv8i8(
+ %0,
+ %1,
+i64 %2)
+
+  ret  %a
+}
+
+declare  @llvm.riscv.vadd.mask.nxv8i8.nxv8i8(
+  ,
+  ,
+  ,
+  ,
+  i64, i64);
+
+define  @intrinsic_vadd_mask_tu( %0,  %1,  %2,  %3, i64 %4) nounwind {
+; CHECK-LABEL: intrinsic_vadd_mask_tu:
+; CHECK:   # %bb.0: # %entry
+; CHECK-NEXT:vsetivli zero, 3, e8, m1, tu, mu
+; CHECK-NEXT:vadd.vv v8, v9, v10, v0.t
+; CHECK-NEXT:jalr zero, 0(ra)
+entry:
+  %a = call  @llvm.riscv.vadd.mask.nxv8i8.nxv8i8(
+ %0,
+ %1,
+ %2,
+ %3,
+i64 %4, i64 0)
+
+  ret  %a
+}
+
+define  @intrinsic_vadd_mask_ta( %0,  %1,  %2,  %3, i64 %4) nounwind {
+; CHECK-LABEL: intrinsic_vadd_mask_ta:
+; CHECK:   # %bb.0: # %entry
+; CHECK-NEXT:vsetivli zero, 3, e8, m1, ta, mu
+; CHECK-NEXT:vadd.vv v8, v9, v10, v0.t
+; CHECK-NEXT:jalr zero, 0(ra)
+entry:
+  %a = call  @llvm.riscv.vadd.mask.nxv8i8.nxv8i8(
+ %0,
+ %1,
+ %2,
+ %3,
+i64 %4, i64 1)
+
+  ret  %a
+}
+
Index: llvm/lib/Target/RISCV/RISCVInstrInfoVVLPatterns.td
===
--- llvm/lib/Target/RISCV/RISCVInstrInfoVVLPatterns.td
+++ llvm/lib/Target/RISCV/RISCVInstrInfoVVLPatterns.td
@@ -22,6 +22,9 @@
 // Helpers to define the VL patterns.
 //===--===//
 
+defvar TAIL_AGNOSTIC = 0;
+defvar TAIL_UNDISTURBED = 1;
+
 def SDT_RISCVVLE_VL : SDTypeProfile<1, 2, [SDTCisVec<0>, SDTCisPtrTy<1>,
SDTCisVT<2, XLenVT>]>;
 def SDT_RISCVVSE_VL : SDTypeProfile<0, 3, [SDTCisVec<0>, SDTCisPtrTy<1>,
@@ -266,7 +269,7 @@
  (result_type (IMPLICIT_DEF)),
  op_reg_class:$rs1,
  op_reg_class:$rs2,
- VMV0:$vm, GPR:$vl, sew)>;
+ VMV0:$vm, GPR:$vl, sew, TAIL_AGNOSTIC)>;
 }
 
 multiclass VPatBinaryVL_XI;
+ VMV0:$vm, GPR:$vl, sew, TAIL_AGNOSTIC)>;
 }
 
 multiclass VPatBinaryVL_VV_VX {
@@ -604,7 +607,7 @@
   VLOpFrag),
 (!cast("PseudoVRSUB_VX_"# vti.LMul.MX#"_MASK")
  (vti.Vector (IMPLICIT_DEF)), vti.RegClass:$rs1, GPR:$rs2,
- VMV0:$vm, GPR:$vl, vti.Log2SEW)>;
+ VMV0:$vm, GPR:$vl, vti.Log2SEW, TAIL_AGNOSTIC)>;
   def : Pat<(riscv_sub_vl (vti.Vector (SplatPat_simm5 simm5:$rs2)),
   (vti.Vector vti.RegClass:$rs1), (vti.Mask true_mask),
   VLOpFrag),
@@ -615,7 +618,7 @@
   VLOpFrag),
 (!cast("PseudoVRSUB_VI_"# vti.LMul.MX#"_MASK")
  (vti.Vector (IMPLICIT_DEF)), vti.RegClass:$rs1, simm5:$rs2,
- VMV0:$vm, GPR:$vl, vti.Log2SEW)>;
+ VMV0:$vm, GPR:$vl, vti.Log2SEW, TAIL_AGNOSTIC)>;
 }
 
 // 12.3. Vector Integer Extension
@@ -1210,7 +1213,7 @@
   VLOpFrag)),
 (!cast("PseudoVRGATHER_VV_"# vti.LMul.MX#"_MASK")
  vti.RegClass:$merge, vti.RegClass:$rs2, vti.RegClass:$rs1,
- vti.Mask:$vm, GPR:$vl, vti.Log2SEW)>;
+ vti.Mask:$vm, GPR:$vl, vti.Log2SEW, TAIL_AGNOSTIC)>;
 
   // emul = lmul * 16 / sew
   defvar vlmul = vti.LMul;
@@ -1237,7 +1240,7 @@
 VLOpFrag)),
   (!cast(inst#"_MASK")
vti.RegClass:$merge, vti.

[PATCH] D105421: [analyzer] Handle << operator for std::unique_ptr

[Valeriy Savchenko via Phabricator via cfe-commits]

vsavchenko accepted this revision.
vsavchenko added a comment.

In D105421#2873458 , @RedDocMD wrote:

> Removed stupid mistakes

No need for self deprecation here!  Thanks for addressing these!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105421

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

[PATCH] D105629: [TSan] Add SystemZ support

[Ilya Leoshkevich via Phabricator via cfe-commits]

iii added inline comments.



Comment at: compiler-rt/lib/tsan/rtl/tsan_platform_posix.cpp:123
+#if defined(__s390x__)
+  ProtectRange(HiAppMemEnd(), 0xf000ull);
+#endif

uweigand wrote:
> Did you test this on older kernels without 5-level page table support?   I 
> believe the allocation / mprotect may fail on those ...
No, not really. Would it make sense to probe here? E.g. first try 
0xf000, then 0x20. Or is there a way to query 
user_addr_max() / TASK_SIZE_MAX / TASK_SIZE?



Comment at: compiler-rt/lib/tsan/rtl/tsan_rtl_s390x.S:22
+  CFI_REL_OFFSET(%r2, R2_REL_OFFSET)
+  CFI_REL_OFFSET(%r3, R3_REL_OFFSET)
+  stmg %r14, %r15, R14_REL_OFFSET(%r15)

uweigand wrote:
> Do we need CFI for r2/r3 ?  Those are call-clobbered any cannot be unwound 
> normally anyway ...
I'm not quite sure, but glibc does this (e.g. in 
sysdeps/s390/s390-64/dl-trampoline.h), so I figured I'll do this here as well 
just in case.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105629

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

[PATCH] D105690: [RISCV] Rename assembler mnemonic of unordered floating-point reductions for v1.0-rc change

[Luís Marques via Phabricator via cfe-commits]

luismarques added inline comments.



Comment at: llvm/test/CodeGen/RISCV/rvv/vfredusum-rv32.ll:1
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc -mtriple=riscv32 -mattr=+experimental-v,+d,+experimental-zfh 
-verify-machineinstrs \

frasercrmck wrote:
> Just to check - was this renaming done with `git mv`? Phabricator suggests 
> that `vfredsum-rv32.ll` was deleted and this was added, which would be worse 
> for the git history. It might be a phabricator quirk, though.
AFAIK `git mv` doesn't do anything in particular to track renames. File renames 
are automatically detected based on the added and removed content, which means 
that if there are also changes to the content that detection might fail, and 
here the instruction renames did cause a lot of changes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105690

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

[PATCH] D105881: [flang][driver] Switch to `BoolFOption` for boolean options

[Andrzej Warzynski via Phabricator via cfe-commits]

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

For boolean options, e.g. `-fxor-operator/-fno-xor-operator`, we ought
to be using TableGen multiclasses. This way, we only have to write one
definition to have both forms auto-generated. This patch refactors all of
Flang's boolean options to use the `BoolFOption` multiclass.

>From what I can see, `BoolFOption` does not support `DocString` and one
of our boolean options did specify a `DocString`. As we don't use these
fields in Flang just yet, deleting it seemed like the right approach at
this stage. We still have the "help" text to provides a brief
description.

This patch also defines a dummy `KeyPathAndMacro` to be used by Flang.
Again, we don't use this field on Flang just yet, so we defined this
dummy instance to avoid creating new TableGen multiclasses for boolean
options.

With the new approach, "empty" help text is now replaced with an empty
string. When running `flang-new --help`, that's considered as non-empty
help messages, which is then printed. This means that with this patch,
`flang-new --help` will start printing e.g. `-fno-backslash`, even
though there is no actual help text to print for this option (apart from
the emtpy string "").


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D105881

Files:
  clang/include/clang/Driver/Options.td
  flang/lib/Frontend/CompilerInvocation.cpp
  flang/test/Driver/driver-help-hidden.f90
  flang/test/Driver/driver-help.f90

Index: flang/test/Driver/driver-help.f90
===
--- flang/test/Driver/driver-help.f90
+++ flang/test/Driver/driver-help.f90
@@ -40,7 +40,12 @@
 ! HELP-NEXT:Specify where to find the compiled intrinsic modules
 ! HELP-NEXT: -flarge-sizes  Use INTEGER(KIND=8) for the result type in size-related intrinsics
 ! HELP-NEXT: -flogical-abbreviations Enable logical abbreviations
+! HELP-NEXT: -fno-backslash
 ! HELP-NEXT: -fno-color-diagnostics Disable colors in diagnostics
+! HELP-NEXT: -fno-implicit-none
+! HELP-NEXT: -fno-logical-abbreviations
+! HELP-NEXT:Disable logical abbreviations
+! HELP-NEXT: -fno-xor-operator  Disable .XOR. as a synonym of .NEQV.
 ! HELP-NEXT: -fopenacc  Enable OpenACC
 ! HELP-NEXT: -fopenmp   Parse OpenMP pragmas and generate parallel code.
 ! HELP-NEXT: -fxor-operator Enable .XOR. as a synonym of .NEQV.
@@ -68,6 +73,8 @@
 ! HELP-FC1-NEXT: -E Only run the preprocessor
 ! HELP-FC1-NEXT: -falternative-parameter-statement
 ! HELP-FC1-NEXT: Enable the old style PARAMETER statement
+! HELP-FC1-NEXT: -fanalyzed-objects-for-unparse
+! HELP-FC1-NEXT:Use the analyzed objects when unparsing
 ! HELP-FC1-NEXT: -fbackslashSpecify that backslash in string introduces an escape character
 ! HELP-FC1-NEXT: -fdebug-dump-all   Dump symbols and the parse tree after the semantic checks
 ! HELP-FC1-NEXT: -fdebug-dump-parse-tree-no-sema
@@ -103,6 +110,11 @@
 ! HELP-FC1-NEXT: -flogical-abbreviations Enable logical abbreviations
 ! HELP-FC1-NEXT: -fno-analyzed-objects-for-unparse
 ! HELP-FC1-NEXT:Do not use the analyzed objects when unparsing
+! HELP-FC1-NEXT: -fno-backslash
+! HELP-FC1-NEXT: -fno-implicit-none
+! HELP-FC1-NEXT: -fno-logical-abbreviations
+! HELP-FC1-NEXT:Disable logical abbreviations
+! HELP-FC1-NEXT: -fno-xor-operator  Disable .XOR. as a synonym of .NEQV.
 ! HELP-FC1-NEXT: -fopenacc  Enable OpenACC
 ! HELP-FC1-NEXT: -fopenmp   Parse OpenMP pragmas and generate parallel code.
 ! HELP-FC1-NEXT: -fxor-operator Enable .XOR. as a synonym of .NEQV.
Index: flang/test/Driver/driver-help-hidden.f90
===
--- flang/test/Driver/driver-help-hidden.f90
+++ flang/test/Driver/driver-help-hidden.f90
@@ -40,7 +40,12 @@
 ! CHECK-NEXT:Specify where to find the compiled intrinsic modules
 ! CHECK-NEXT: -flarge-sizes  Use INTEGER(KIND=8) for the result type in size-related intrinsics
 ! CHECK-NEXT: -flogical-abbreviations Enable logical abbreviations
+! CHECK-NEXT: -fno-backslash
 ! CHECK-NEXT: -fno-color-diagnostics Disable colors in diagnostics
+! CHECK-NEXT: -fno-implicit-none
+! CHECK-NEXT: -fno-logical-abbreviations
+! CHECK-NEXT:Disable logical abbreviations
+! CHECK-NEXT: -fno-xor-operator  Disable .XOR. as a synonym of .NEQV.
 ! CHECK-NEXT: -fopenacc  Enable OpenACC
 ! CHECK-NEXT: -fopenmp   Parse OpenMP pragmas and generate parallel code.
 ! CHECK-NEXT: -fxor-operator Enable .XOR. as a synonym of .NEQV.
Index: flang/lib/Frontend/CompilerInvocation.cpp
=

[PATCH] D105881: [flang][driver] Switch to `BoolFOption` for boolean options

[Andrzej Warzynski via Phabricator via cfe-commits]

awarzynski added inline comments.



Comment at: clang/include/clang/Driver/Options.td:246
 
+class EmptyKPM : KeyPathAndMacro<"", "", ""> {}
 class DiagnosticOpts

@jansvoboda11 , is this the right approach here? I'd like use `BoolFOption` in 
Flang, but we don't need `KeyPathAndMacro` just yet. We may do in the future.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105881

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

[PATCH] D102875: [PowerPC] Add PowerPC compare and multiply related builtins and instrinsics for XL compatibility

[Nemanja Ivanovic via Phabricator via cfe-commits]

nemanjai added inline comments.



Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:9754
   "this builtin is only valid on POWER7 or later CPUs">;
+def err_ppc_builtin_only_on_pwr9 : Error<
+  "this builtin is only valid on POWER9 or later CPUs">;

No longer required after https://reviews.llvm.org/D105501



Comment at: clang/lib/Sema/SemaChecking.cpp:3356
+  case PPC::BI__builtin_ppc_maddld:
+return SemaFeatureCheck(*this, TheCall, "power9-vector",
+diag::err_ppc_builtin_only_on_pwr9);

NeHuang wrote:
> amyk wrote:
> > This is just a question. 
> > Is `power9-vector` the correct feature check in these cases? Does it matter 
> > if these are not vector instructions?
> yeah, we planned using this feature to do the sema check for `pwr9` only (or 
> later cpus) builtins.
Now that https://reviews.llvm.org/D105501 is approved, please change this to
`isa-v30-instructions` please.



Comment at: llvm/include/llvm/IR/IntrinsicsPowerPC.td:1532
+  : GCCBuiltin<"__builtin_ppc_cmprb">,
+Intrinsic<[llvm_i32_ty], [llvm_i32_ty, llvm_i32_ty, llvm_i32_ty], 
[IntrNoMem]>;
+  def int_ppc_setb

Shouldn't operand 0 be marked as an immediate here?



Comment at: llvm/lib/Target/PowerPC/PPCInstrInfo.td:5263
+let Predicates = [IsISA3_0] in {
+def : Pat<(i32 (int_ppc_cmprb i32:$a, i32:$b, i32:$c)),
+  (i32 (SETB (CMPRB imm:$a, $b, $c)))>;

Shouldn't `$a` be a `u1imm`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102875

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

[clang] 78463eb - [OpenCL] Add support of __opencl_c_generic_address_space feature macro

[Anton Zabaznov via cfe-commits]

Author: Anton Zabaznov
Date: 2021-07-13T13:14:10+03:00
New Revision: 78463ebde2f8a1b8ce984c1ae7c6da0c2d323005

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

LOG: [OpenCL] Add support of __opencl_c_generic_address_space feature macro

Reviewed By: Anastasia

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

Added: 


Modified: 
clang/lib/Basic/TargetInfo.cpp
clang/lib/Parse/ParseDecl.cpp
clang/test/CodeGenOpenCL/address-spaces-conversions.cl
clang/test/CodeGenOpenCL/address-spaces-mangling.cl
clang/test/CodeGenOpenCL/address-spaces.cl
clang/test/CodeGenOpenCL/amdgpu-sizeof-alignof.cl
clang/test/CodeGenOpenCL/overload.cl
clang/test/SemaOpenCL/address-spaces-conversions-cl2.0.cl
clang/test/SemaOpenCL/address-spaces.cl

Removed: 




diff  --git a/clang/lib/Basic/TargetInfo.cpp b/clang/lib/Basic/TargetInfo.cpp
index 88086fa2fed74..b647a2fb8a679 100644
--- a/clang/lib/Basic/TargetInfo.cpp
+++ b/clang/lib/Basic/TargetInfo.cpp
@@ -396,6 +396,19 @@ void TargetInfo::adjust(DiagnosticsEngine &Diags, 
LangOptions &Opts) {
 HalfFormat = &llvm::APFloat::IEEEhalf();
 FloatFormat = &llvm::APFloat::IEEEsingle();
 LongDoubleFormat = &llvm::APFloat::IEEEquad();
+
+// OpenCL C v3.0 s6.7.5 - The generic address space requires support for
+// OpenCL C 2.0 or OpenCL C 3.0 with the __opencl_c_generic_address_space
+// feature
+// FIXME: OpenCLGenericAddressSpace is also defined in setLangDefaults()
+// for OpenCL C 2.0 but with no access to target capabilities. Target
+// should be immutable once created and thus this language option needs
+// to be defined only once.
+if (Opts.OpenCLVersion >= 300) {
+  const auto &OpenCLFeaturesMap = getSupportedOpenCLOpts();
+  Opts.OpenCLGenericAddressSpace = hasFeatureEnabled(
+  OpenCLFeaturesMap, "__opencl_c_generic_address_space");
+}
   }
 
   if (Opts.DoubleSize) {

diff  --git a/clang/lib/Parse/ParseDecl.cpp b/clang/lib/Parse/ParseDecl.cpp
index c1f20b2e42a22..f4f5f461e3b67 100644
--- a/clang/lib/Parse/ParseDecl.cpp
+++ b/clang/lib/Parse/ParseDecl.cpp
@@ -4072,6 +4072,8 @@ void Parser::ParseDeclarationSpecifiers(DeclSpec &DS,
 case tok::kw___generic:
   // generic address space is introduced only in OpenCL v2.0
   // see OpenCL C Spec v2.0 s6.5.5
+  // OpenCL v3.0 introduces __opencl_c_generic_address_space
+  // feature macro to indicate if generic address space is supported
   if (!Actions.getLangOpts().OpenCLGenericAddressSpace) {
 DiagID = diag::err_opencl_unknown_type_specifier;
 PrevSpec = Tok.getIdentifierInfo()->getNameStart();

diff  --git a/clang/test/CodeGenOpenCL/address-spaces-conversions.cl 
b/clang/test/CodeGenOpenCL/address-spaces-conversions.cl
index cd3099e0a1a4f..8fdb46184bed6 100644
--- a/clang/test/CodeGenOpenCL/address-spaces-conversions.cl
+++ b/clang/test/CodeGenOpenCL/address-spaces-conversions.cl
@@ -1,5 +1,7 @@
 // RUN: %clang_cc1 %s -triple x86_64-unknown-linux-gnu -O0 
-ffake-address-space-map -cl-std=CL2.0 -emit-llvm -o - | FileCheck %s
+// RUN: %clang_cc1 %s -triple x86_64-unknown-linux-gnu -O0 
-ffake-address-space-map -cl-std=CL3.0 
-cl-ext=+__opencl_c_generic_address_space -emit-llvm -o - | FileCheck %s
 // RUN: %clang_cc1 %s -triple x86_64-unknown-linux-gnu -O0 -cl-std=CL2.0 
-emit-llvm -o - | FileCheck --check-prefix=CHECK-NOFAKE %s
+// RUN: %clang_cc1 %s -triple x86_64-unknown-linux-gnu -O0 -cl-std=CL3.0 
-cl-ext=+__opencl_c_generic_address_space -emit-llvm -o - | FileCheck 
--check-prefix=CHECK-NOFAKE %s
 // When -ffake-address-space-map is not used, all addr space mapped to 0 for 
x86_64.
 
 // test that we generate address space casts everywhere we need conversions of

diff  --git a/clang/test/CodeGenOpenCL/address-spaces-mangling.cl 
b/clang/test/CodeGenOpenCL/address-spaces-mangling.cl
index 50622f0991430..b46834c2a678c 100644
--- a/clang/test/CodeGenOpenCL/address-spaces-mangling.cl
+++ b/clang/test/CodeGenOpenCL/address-spaces-mangling.cl
@@ -2,10 +2,14 @@
 // RUN: %clang_cc1 %s -cl-std=CL2.0 -ffake-address-space-map 
-faddress-space-map-mangling=yes -triple %itanium_abi_triple -emit-llvm -o - | 
FileCheck -check-prefixes="ASMANG,ASMANG20" %s
 // RUN: %clang_cc1 %s -ffake-address-space-map -faddress-space-map-mangling=no 
-triple %itanium_abi_triple -emit-llvm -o - | FileCheck 
-check-prefixes="NOASMANG,NOASMANG10" %s
 // RUN: %clang_cc1 %s -cl-std=CL2.0 -ffake-address-space-map 
-faddress-space-map-mangling=no -triple %itanium_abi_triple -emit-llvm -o - | 
FileCheck -check-prefixes="NOASMANG,NOASMANG20" %s
+// RUN: %clang_cc1 %s -cl-std=CL3.0 -cl-std=CL3.0 
-cl-ext=+__opencl_c_generic_address_space -ffake-address-space-map 
-faddress-space-map-mangling=no -triple %itan

[PATCH] D103401: [OpenCL] Add support of __opencl_c_generic_address_space feature macro

[Anton Zabaznov via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG78463ebde2f8: [OpenCL] Add support of 
__opencl_c_generic_address_space feature macro (authored by azabaznov).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103401

Files:
  clang/lib/Basic/TargetInfo.cpp
  clang/lib/Parse/ParseDecl.cpp
  clang/test/CodeGenOpenCL/address-spaces-conversions.cl
  clang/test/CodeGenOpenCL/address-spaces-mangling.cl
  clang/test/CodeGenOpenCL/address-spaces.cl
  clang/test/CodeGenOpenCL/amdgpu-sizeof-alignof.cl
  clang/test/CodeGenOpenCL/overload.cl
  clang/test/SemaOpenCL/address-spaces-conversions-cl2.0.cl
  clang/test/SemaOpenCL/address-spaces.cl

Index: clang/test/SemaOpenCL/address-spaces.cl
===
--- clang/test/SemaOpenCL/address-spaces.cl
+++ clang/test/SemaOpenCL/address-spaces.cl
@@ -1,5 +1,6 @@
 // RUN: %clang_cc1 %s -verify -pedantic -fsyntax-only
 // RUN: %clang_cc1 %s -cl-std=CL2.0 -verify -pedantic -fsyntax-only
+// RUN: %clang_cc1 %s -cl-std=CL3.0 -cl-ext=+__opencl_c_generic_address_space -verify -pedantic -fsyntax-only
 // RUN: %clang_cc1 %s -cl-std=clc++ -verify -pedantic -fsyntax-only
 
 __constant int ci = 1;
Index: clang/test/SemaOpenCL/address-spaces-conversions-cl2.0.cl
===
--- clang/test/SemaOpenCL/address-spaces-conversions-cl2.0.cl
+++ clang/test/SemaOpenCL/address-spaces-conversions-cl2.0.cl
@@ -4,6 +4,9 @@
 // RUN: %clang_cc1 %s -ffake-address-space-map -verify -pedantic -fsyntax-only -DCONSTANT -cl-std=clc++
 // RUN: %clang_cc1 %s -ffake-address-space-map -verify -pedantic -fsyntax-only -DGLOBAL -cl-std=clc++
 // RUN: %clang_cc1 %s -ffake-address-space-map -verify -pedantic -fsyntax-only -DGENERIC -cl-std=clc++
+// RUN: %clang_cc1 %s -ffake-address-space-map -verify -pedantic -fsyntax-only -DCONSTANT -cl-std=CL3.0 -cl-ext=+__opencl_c_generic_address_space
+// RUN: %clang_cc1 %s -ffake-address-space-map -verify -pedantic -fsyntax-only -DGLOBAL -cl-std=CL3.0 -cl-ext=+__opencl_c_generic_address_space
+// RUN: %clang_cc1 %s -ffake-address-space-map -verify -pedantic -fsyntax-only -DGENERIC -cl-std=CL3.0 -cl-ext=+__opencl_c_generic_address_space
 
 /* OpenCLC v2.0 adds a set of restrictions for conversions between pointers to
 *  different address spaces, mainly described in Sections 6.5.5 and 6.5.6.
@@ -17,6 +20,8 @@
 *  case), and __constant, that should cover all program paths for CL address
 *  space conversions used in initialisations, assignments, casts, comparisons
 *  and arithmetic operations.
+*
+*  OpenCLC v3.0 supports generic address if __opencl_c_generic_address_space feature is supported
 */
 
 #ifdef GENERIC
Index: clang/test/CodeGenOpenCL/overload.cl
===
--- clang/test/CodeGenOpenCL/overload.cl
+++ clang/test/CodeGenOpenCL/overload.cl
@@ -1,4 +1,5 @@
 // RUN: %clang_cc1 -cl-std=CL2.0 -emit-llvm -o - -triple spir-unknown-unknown %s | FileCheck %s
+// RUN: %clang_cc1 -cl-std=CL3.0 -cl-ext=+__opencl_c_generic_address_space -emit-llvm -o - -triple spir-unknown-unknown %s | FileCheck %s
 
 typedef short short4 __attribute__((ext_vector_type(4)));
 
Index: clang/test/CodeGenOpenCL/amdgpu-sizeof-alignof.cl
===
--- clang/test/CodeGenOpenCL/amdgpu-sizeof-alignof.cl
+++ clang/test/CodeGenOpenCL/amdgpu-sizeof-alignof.cl
@@ -5,6 +5,18 @@
 // RUN: %clang_cc1 -triple amdgcn---amdgizcl -cl-std=CL1.2 %s -emit-llvm -o - | FileCheck %s
 // RUN: %clang_cc1 -triple amdgcn---amdgizcl -cl-std=CL2.0 %s -emit-llvm -o - | FileCheck %s
 
+// RUN: %clang_cc1 -triple r600 -cl-std=CL3.0 %s -emit-llvm -o - | FileCheck %s
+// RUN: %clang_cc1 -triple amdgcn-mesa-mesa3d -cl-std=CL3.0 %s -emit-llvm -o - | FileCheck %s
+// RUN: %clang_cc1 -triple amdgcn---opencl -cl-std=CL3.0 %s -emit-llvm -o - | FileCheck %s
+// RUN: %clang_cc1 -triple amdgcn---amdgizcl -cl-std=CL3.0 %s -emit-llvm -o - | FileCheck %s
+// RUN: %clang_cc1 -triple amdgcn-mesa-mesa3d -cl-ext=+__opencl_c_generic_address_space -cl-std=CL3.0 %s -emit-llvm -o - | FileCheck %s
+// RUN: %clang_cc1 -triple amdgcn---opencl -cl-ext=+__opencl_c_generic_address_space -cl-std=CL3.0 %s -emit-llvm -o - | FileCheck %s
+// RUN: %clang_cc1 -triple amdgcn---amdgizcl -cl-ext=+__opencl_c_generic_address_space -cl-std=CL3.0 %s -emit-llvm -o - | FileCheck %s
+// RUN: %clang_cc1 -triple r600 -cl-ext=+cl_khr_fp64,+__opencl_c_fp64 -cl-std=CL3.0 %s -emit-llvm -o - | FileCheck %s
+// RUN: %clang_cc1 -triple amdgcn-mesa-mesa3d -cl-ext=+cl_khr_fp64,+__opencl_c_fp64 -cl-std=CL3.0 %s -emit-llvm -o - | FileCheck %s
+// RUN: %clang_cc1 -triple amdgcn---opencl -cl-ext=+cl_khr_fp64,+__opencl_c_fp64 -cl-std=CL3.0 %s -emit-llvm -o - | FileCheck %s
+// RUN: %clang_cc1 -triple amdgcn---amdgizcl -cl-ext=+cl_khr_fp64,+__op

[PATCH] D105629: [TSan] Add SystemZ support

[Ulrich Weigand via Phabricator via cfe-commits]

uweigand added inline comments.



Comment at: compiler-rt/lib/tsan/rtl/tsan_platform_posix.cpp:123
+#if defined(__s390x__)
+  ProtectRange(HiAppMemEnd(), 0xf000ull);
+#endif

iii wrote:
> uweigand wrote:
> > Did you test this on older kernels without 5-level page table support?   I 
> > believe the allocation / mprotect may fail on those ...
> No, not really. Would it make sense to probe here? E.g. first try 
> 0xf000, then 0x20. Or is there a way to query 
> user_addr_max() / TASK_SIZE_MAX / TASK_SIZE?
I don't know of any way to query this.  You can simply do the allocation in 
stages (first to the top of the three-pagetable range, then four, then five), 
and ignore failures.   As an unfortunate side effect this will force the kernel 
to allocate five levels of page tables even when unnecessary, but I don't think 
there's anything we can do about that.



Comment at: compiler-rt/lib/tsan/rtl/tsan_rtl_s390x.S:22
+  CFI_REL_OFFSET(%r2, R2_REL_OFFSET)
+  CFI_REL_OFFSET(%r3, R3_REL_OFFSET)
+  stmg %r14, %r15, R14_REL_OFFSET(%r15)

iii wrote:
> uweigand wrote:
> > Do we need CFI for r2/r3 ?  Those are call-clobbered any cannot be unwound 
> > normally anyway ...
> I'm not quite sure, but glibc does this (e.g. in 
> sysdeps/s390/s390-64/dl-trampoline.h), so I figured I'll do this here as well 
> just in case.
Huh.   Well I guess it doesn't hurt either way.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105629

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

[PATCH] D105629: [TSan] Add SystemZ support

[Ilya Leoshkevich via Phabricator via cfe-commits]

iii added inline comments.



Comment at: compiler-rt/lib/tsan/rtl/tsan_platform_posix.cpp:123
+#if defined(__s390x__)
+  ProtectRange(HiAppMemEnd(), 0xf000ull);
+#endif

uweigand wrote:
> iii wrote:
> > uweigand wrote:
> > > Did you test this on older kernels without 5-level page table support?   
> > > I believe the allocation / mprotect may fail on those ...
> > No, not really. Would it make sense to probe here? E.g. first try 
> > 0xf000, then 0x20. Or is there a way to query 
> > user_addr_max() / TASK_SIZE_MAX / TASK_SIZE?
> I don't know of any way to query this.  You can simply do the allocation in 
> stages (first to the top of the three-pagetable range, then four, then five), 
> and ignore failures.   As an unfortunate side effect this will force the 
> kernel to allocate five levels of page tables even when unnecessary, but I 
> don't think there's anything we can do about that.
3-level page table limit is quite below the application range defined in this 
series (0x400 < 0xc000). Are there any relevant kernels that do 
not support 4-level page tables? I tried to search the git history, and it 
looks as if even 2.6 ones have it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105629

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

[PATCH] D105629: [TSan] Add SystemZ support

[Ulrich Weigand via Phabricator via cfe-commits]

uweigand added inline comments.



Comment at: compiler-rt/lib/tsan/rtl/tsan_platform_posix.cpp:123
+#if defined(__s390x__)
+  ProtectRange(HiAppMemEnd(), 0xf000ull);
+#endif

iii wrote:
> uweigand wrote:
> > iii wrote:
> > > uweigand wrote:
> > > > Did you test this on older kernels without 5-level page table support?  
> > > >  I believe the allocation / mprotect may fail on those ...
> > > No, not really. Would it make sense to probe here? E.g. first try 
> > > 0xf000, then 0x20. Or is there a way to query 
> > > user_addr_max() / TASK_SIZE_MAX / TASK_SIZE?
> > I don't know of any way to query this.  You can simply do the allocation in 
> > stages (first to the top of the three-pagetable range, then four, then 
> > five), and ignore failures.   As an unfortunate side effect this will force 
> > the kernel to allocate five levels of page tables even when unnecessary, 
> > but I don't think there's anything we can do about that.
> 3-level page table limit is quite below the application range defined in this 
> series (0x400 < 0xc000). Are there any relevant kernels that 
> do not support 4-level page tables? I tried to search the git history, and it 
> looks as if even 2.6 ones have it.
Ah, right.  Yes, we can certainly assume 4-level support at this point.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105629

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

[PATCH] D105756: [clang] C++98 implicit moves are back with a vengeance

[Matheus Izvekov via Phabricator via cfe-commits]

mizvekov updated this revision to Diff 358224.
mizvekov edited the summary of this revision.
mizvekov added a comment.

- Fix minor nits.
- Please Dijkstra.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105756

Files:
  clang/lib/Sema/SemaStmt.cpp
  clang/test/CXX/class/class.init/class.copy.elision/p3.cpp
  clang/test/SemaCXX/conversion-function.cpp
  clang/test/SemaObjCXX/block-capture.mm

Index: clang/test/SemaObjCXX/block-capture.mm
===
--- clang/test/SemaObjCXX/block-capture.mm
+++ clang/test/SemaObjCXX/block-capture.mm
@@ -14,6 +14,10 @@
 };
 TEST(CopyOnly); // cxx2b-error {{no matching constructor}}
 
+// Both ConstCopyOnly and NonConstCopyOnly are
+// "pure" C++98 tests (pretend 'delete' means 'private').
+// However we may extend implicit moves into C++98, we must make sure the
+// results in these are not changed.
 struct ConstCopyOnly {
   ConstCopyOnly();
   ConstCopyOnly(ConstCopyOnly &) = delete; // cxx98-note {{marked deleted here}}
@@ -31,51 +35,51 @@
 struct CopyNoMove {
   CopyNoMove();
   CopyNoMove(CopyNoMove &);
-  CopyNoMove(CopyNoMove &&) = delete; // cxx11_2b-note {{marked deleted here}}
+  CopyNoMove(CopyNoMove &&) = delete; // cxx98_2b-note {{marked deleted here}}
 };
-TEST(CopyNoMove); // cxx11_2b-error {{call to deleted constructor}}
+TEST(CopyNoMove); // cxx98_2b-error {{call to deleted constructor}}
 
 struct MoveOnly {
   MoveOnly();
-  MoveOnly(MoveOnly &) = delete; // cxx98-note {{marked deleted here}}
+  MoveOnly(MoveOnly &) = delete;
   MoveOnly(MoveOnly &&);
 };
-TEST(MoveOnly); // cxx98-error {{call to deleted constructor}}
+TEST(MoveOnly);
 
 struct NoCopyNoMove {
   NoCopyNoMove();
-  NoCopyNoMove(NoCopyNoMove &) = delete;  // cxx98-note {{marked deleted here}}
-  NoCopyNoMove(NoCopyNoMove &&) = delete; // cxx11_2b-note {{marked deleted here}}
+  NoCopyNoMove(NoCopyNoMove &) = delete;
+  NoCopyNoMove(NoCopyNoMove &&) = delete; // cxx98_2b-note {{marked deleted here}}
 };
 TEST(NoCopyNoMove); // cxx98_2b-error {{call to deleted constructor}}
 
 struct ConvertingRVRef {
   ConvertingRVRef();
-  ConvertingRVRef(ConvertingRVRef &) = delete; // cxx98-note {{marked deleted here}}
+  ConvertingRVRef(ConvertingRVRef &) = delete;
 
   struct X {};
   ConvertingRVRef(X &&);
   operator X() const & = delete;
   operator X() &&;
 };
-TEST(ConvertingRVRef); // cxx98-error {{call to deleted constructor}}
+TEST(ConvertingRVRef);
 
 struct ConvertingCLVRef {
   ConvertingCLVRef();
   ConvertingCLVRef(ConvertingCLVRef &);
 
   struct X {};
-  ConvertingCLVRef(X &&); // cxx11_2b-note {{passing argument to parameter here}}
+  ConvertingCLVRef(X &&); // cxx98_2b-note {{passing argument to parameter here}}
   operator X() const &;
-  operator X() && = delete; // cxx11_2b-note {{marked deleted here}}
+  operator X() && = delete; // cxx98_2b-note {{marked deleted here}}
 };
-TEST(ConvertingCLVRef); // cxx11_2b-error {{invokes a deleted function}}
+TEST(ConvertingCLVRef); // cxx98_2b-error {{invokes a deleted function}}
 
 struct SubSubMove {};
 struct SubMove : SubSubMove {
   SubMove();
-  SubMove(SubMove &) = delete; // cxx98-note {{marked deleted here}}
+  SubMove(SubMove &) = delete;
 
   SubMove(SubSubMove &&);
 };
-TEST(SubMove); // cxx98-error {{call to deleted constructor}}
+TEST(SubMove);
Index: clang/test/SemaCXX/conversion-function.cpp
===
--- clang/test/SemaCXX/conversion-function.cpp
+++ clang/test/SemaCXX/conversion-function.cpp
@@ -120,7 +120,9 @@
   char ch = a;  // OK. calls Yb::operator char();
 }
 
-// Test conversion + copy construction.
+// Test conversion + copy construction. This is a pure C++98 test.
+// However we may extend implicit moves into C++98, we must make sure the
+// result here is not changed.
 class AutoPtrRef { };
 
 class AutoPtr {
Index: clang/test/CXX/class/class.init/class.copy.elision/p3.cpp
===
--- clang/test/CXX/class/class.init/class.copy.elision/p3.cpp
+++ clang/test/CXX/class/class.init/class.copy.elision/p3.cpp
@@ -7,11 +7,11 @@
 struct A1 {
   A1();
   A1(const A1 &);
-  A1(A1 &&) = delete; // cxx11_2b-note {{'A1' has been explicitly marked deleted here}}
+  A1(A1 &&) = delete; // expected-note {{'A1' has been explicitly marked deleted here}}
 };
 A1 test1() {
   A1 a;
-  return a; // cxx11_2b-error {{call to deleted constructor of 'test_delete_function::A1'}}
+  return a; // expected-error {{call to deleted constructor of 'test_delete_function::A1'}}
 }
 
 struct A2 {
@@ -19,33 +19,33 @@
   A2(const A2 &);
 
 private:
-  A2(A2 &&); // cxx11_2b-note {{declared private here}}
+  A2(A2 &&); // expected-note {{declared private here}}
 };
 A2 test2() {
   A2 a;
-  return a; // cxx11_2b-error {{calling a private constructor of class 'test_delete_function::A2'}}
+  return 

[PATCH] D105756: [clang] C++98 implicit moves are back with a vengeance

[Matheus Izvekov via Phabricator via cfe-commits]

mizvekov added inline comments.



Comment at: clang/test/SemaObjCXX/block-capture.mm:40
 };
-TEST(CopyNoMove); // cxx11_2b-error {{call to deleted constructor}}
+TEST(CopyNoMove); // cxx98_2b-error {{call to deleted constructor}}
 

Quuxplusone wrote:
> `cxx98_2b` is just `expected`, isn't it?
Yeah, but in this test we don't have the "expected" expectancy string.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105756

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

[PATCH] D105526: opencl-c.h: CL3.0 generic address space

[Anastasia Stulova via Phabricator via cfe-commits]

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

LGTM! Thanks!

PS this patch is straightforward and doesn't seem to affect functionality for 
older standards provided that we set `__opencl_c_generic_address_space` 
correctly which is not in the scope of this change.


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

https://reviews.llvm.org/D105526

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

[PATCH] D105754: [PowerPC] Fix L[D|W]ARX Implementation

[Albion Fung via Phabricator via cfe-commits]

Conanap added inline comments.



Comment at: 
llvm/test/CodeGen/PowerPC/builtins-ppc-xlcompat-LoadReserve-StoreCond-64bit-only.ll:18
 entry:
-  %0 = bitcast i64* %a to i8*
-  %1 = tail call i64 @llvm.ppc.ldarx(i8* %0)
-  ret i64 %1
+  %0 = call i64 asm sideeffect "ldarx $0, $1", "=r,*Z,~{memory}"(i64* %a)
+  ret i64 %0

nemanjai wrote:
> This is not the asm that the front end generates. Why would you generate one 
> thing in the front end and then test a different thing in the back end?
I'm not quite sure what you mean by this; the IR output is taken from the `.c` 
test case above.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105754

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

[PATCH] D105754: [PowerPC] Fix L[D|W]ARX Implementation

[Nemanja Ivanovic via Phabricator via cfe-commits]

nemanjai added inline comments.



Comment at: 
llvm/test/CodeGen/PowerPC/builtins-ppc-xlcompat-LoadReserve-StoreCond-64bit-only.ll:18
 entry:
-  %0 = bitcast i64* %a to i8*
-  %1 = tail call i64 @llvm.ppc.ldarx(i8* %0)
-  ret i64 %1
+  %0 = call i64 asm sideeffect "ldarx $0, $1", "=r,*Z,~{memory}"(i64* %a)
+  ret i64 %0

Conanap wrote:
> nemanjai wrote:
> > This is not the asm that the front end generates. Why would you generate 
> > one thing in the front end and then test a different thing in the back end?
> I'm not quite sure what you mean by this; the IR output is taken from the 
> `.c` test case above.
I don't think that is the case.
Above:
`call i64 asm sideeffect "ldarx $0, ${1:y}", "=r,*Z,~{memory}"(i64* %a)`
Here: 
`call i64 asm sideeffect "ldarx $0, $1", "=r,*Z,~{memory}"(i64* %a)`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105754

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

[PATCH] D105754: [PowerPC] Fix L[D|W]ARX Implementation

[Albion Fung via Phabricator via cfe-commits]

Conanap added inline comments.



Comment at: 
llvm/test/CodeGen/PowerPC/builtins-ppc-xlcompat-LoadReserve-StoreCond-64bit-only.ll:18
 entry:
-  %0 = bitcast i64* %a to i8*
-  %1 = tail call i64 @llvm.ppc.ldarx(i8* %0)
-  ret i64 %1
+  %0 = call i64 asm sideeffect "ldarx $0, $1", "=r,*Z,~{memory}"(i64* %a)
+  ret i64 %0

nemanjai wrote:
> Conanap wrote:
> > nemanjai wrote:
> > > This is not the asm that the front end generates. Why would you generate 
> > > one thing in the front end and then test a different thing in the back 
> > > end?
> > I'm not quite sure what you mean by this; the IR output is taken from the 
> > `.c` test case above.
> I don't think that is the case.
> Above:
> `call i64 asm sideeffect "ldarx $0, ${1:y}", "=r,*Z,~{memory}"(i64* %a)`
> Here: 
> `call i64 asm sideeffect "ldarx $0, $1", "=r,*Z,~{memory}"(i64* %a)`
ah I see I missed the modifier.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105754

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

[PATCH] D105754: [PowerPC] Fix L[D|W]ARX Implementation

[Albion Fung via Phabricator via cfe-commits]

Conanap updated this revision to Diff 358028.
Conanap added a comment.

Updated a test case


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105754

Files:
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/test/CodeGen/builtins-ppc-xlcompat-LoadReseve-StoreCond-64bit-only.c
  clang/test/CodeGen/builtins-ppc-xlcompat-LoadReseve-StoreCond.c
  llvm/include/llvm/IR/IntrinsicsPowerPC.td
  llvm/lib/Target/PowerPC/PPCInstr64Bit.td
  llvm/lib/Target/PowerPC/PPCInstrInfo.td
  
llvm/test/CodeGen/PowerPC/builtins-ppc-xlcompat-LoadReserve-StoreCond-64bit-only.ll
  llvm/test/CodeGen/PowerPC/builtins-ppc-xlcompat-LoadReserve-StoreCond.ll

Index: llvm/test/CodeGen/PowerPC/builtins-ppc-xlcompat-LoadReserve-StoreCond.ll
===
--- llvm/test/CodeGen/PowerPC/builtins-ppc-xlcompat-LoadReserve-StoreCond.ll
+++ llvm/test/CodeGen/PowerPC/builtins-ppc-xlcompat-LoadReserve-StoreCond.ll
@@ -12,18 +12,21 @@
 define dso_local signext i32 @test_lwarx(i32* readnone %a) {
 ; CHECK-64-LABEL: test_lwarx:
 ; CHECK-64:   # %bb.0: # %entry
+; CHECK-64-NEXT:#APP
 ; CHECK-64-NEXT:lwarx 3, 0, 3
+; CHECK-64-NEXT:#NO_APP
 ; CHECK-64-NEXT:extsw 3, 3
 ; CHECK-64-NEXT:blr
 ;
 ; CHECK-32-LABEL: test_lwarx:
 ; CHECK-32:   # %bb.0: # %entry
+; CHECK-32-NEXT:#APP
 ; CHECK-32-NEXT:lwarx 3, 0, 3
+; CHECK-32-NEXT:#NO_APP
 ; CHECK-32-NEXT:blr
 entry:
-  %0 = bitcast i32* %a to i8*
-  %1 = tail call i32 @llvm.ppc.lwarx(i8* %0)
-  ret i32 %1
+  %0 = call i32 asm sideeffect "lwarx $0, $1", "=r,*Z,~{memory}"(i32* %a)
+  ret i32 %0
 }
 
 declare i32 @llvm.ppc.stwcx(i8*, i32)
Index: llvm/test/CodeGen/PowerPC/builtins-ppc-xlcompat-LoadReserve-StoreCond-64bit-only.ll
===
--- llvm/test/CodeGen/PowerPC/builtins-ppc-xlcompat-LoadReserve-StoreCond-64bit-only.ll
+++ llvm/test/CodeGen/PowerPC/builtins-ppc-xlcompat-LoadReserve-StoreCond-64bit-only.ll
@@ -10,17 +10,18 @@
 define dso_local i64 @test_ldarx(i64* readnone %a) {
 ; CHECK-LABEL: test_ldarx:
 ; CHECK:   # %bb.0: # %entry
+; CHECK-NEXT:#APP
 ; CHECK-NEXT:ldarx 3, 0, 3
+; CHECK-NEXT:#NO_APP
 ; CHECK-NEXT:blr
 entry:
-  %0 = bitcast i64* %a to i8*
-  %1 = tail call i64 @llvm.ppc.ldarx(i8* %0)
-  ret i64 %1
+  %0 = call i64 asm sideeffect "ldarx $0, ${1:y}", "=r,*Z,~{memory}"(i64* %a)
+  ret i64 %0
 }
 
 declare i32 @llvm.ppc.stdcx(i8*, i64)
-define dso_local i64 @test(i64* %a, i64 %b) {
-; CHECK-LABEL: test:
+define dso_local i64 @test_stdcx(i64* %a, i64 %b) {
+; CHECK-LABEL: test_stdcx:
 ; CHECK:   # %bb.0: # %entry
 ; CHECK-NEXT:stdcx. 4, 0, 3
 ; CHECK-NEXT:mfocrf 3, 128
Index: llvm/lib/Target/PowerPC/PPCInstrInfo.td
===
--- llvm/lib/Target/PowerPC/PPCInstrInfo.td
+++ llvm/lib/Target/PowerPC/PPCInstrInfo.td
@@ -5412,7 +5412,5 @@
 def : Pat<(i64 (bitreverse i64:$A)),
   (OR8 (RLDICR DWBytes7654.DWord, 32, 31), DWBytes3210.DWord)>;
 
-def : Pat<(int_ppc_lwarx ForceXForm:$dst),
-  (LWARX ForceXForm:$dst)>;
 def : Pat<(int_ppc_stwcx ForceXForm:$dst, gprc:$A),
   (STWCX gprc:$A, ForceXForm:$dst)>;
Index: llvm/lib/Target/PowerPC/PPCInstr64Bit.td
===
--- llvm/lib/Target/PowerPC/PPCInstr64Bit.td
+++ llvm/lib/Target/PowerPC/PPCInstr64Bit.td
@@ -1723,5 +1723,3 @@
 
 def : Pat<(int_ppc_stdcx ForceXForm:$dst, g8rc:$A),
   (STDCX g8rc:$A, ForceXForm:$dst)>;
-def : Pat<(int_ppc_ldarx ForceXForm:$dst),
-  (LDARX ForceXForm:$dst)>;
Index: llvm/include/llvm/IR/IntrinsicsPowerPC.td
===
--- llvm/include/llvm/IR/IntrinsicsPowerPC.td
+++ llvm/include/llvm/IR/IntrinsicsPowerPC.td
@@ -1529,9 +1529,5 @@
   def int_ppc_stwcx : GCCBuiltin<"__builtin_ppc_stwcx">,
   Intrinsic<[llvm_i32_ty], [llvm_ptr_ty, llvm_i32_ty],
 [IntrWriteMem]>;
-  def int_ppc_lwarx : GCCBuiltin<"__builtin_ppc_lwarx">,
-  Intrinsic<[llvm_i32_ty], [llvm_ptr_ty], [IntrNoMem]>;
-  def int_ppc_ldarx : GCCBuiltin<"__builtin_ppc_ldarx">,
-  Intrinsic<[llvm_i64_ty], [llvm_ptr_ty], [IntrNoMem]>;
 }
 
Index: clang/test/CodeGen/builtins-ppc-xlcompat-LoadReseve-StoreCond.c
===
--- clang/test/CodeGen/builtins-ppc-xlcompat-LoadReseve-StoreCond.c
+++ clang/test/CodeGen/builtins-ppc-xlcompat-LoadReseve-StoreCond.c
@@ -1,22 +1,20 @@
-// RUN: %clang_cc1 -triple=powerpc-unknown-aix -emit-llvm %s -o - | \
+// RUN: %clang_cc1 -O2 -triple=powerpc-unknown-aix -emit-llvm %s -o - | \
 // RUN: FileCheck %s
-// RUN: %clang_cc1 -triple=powerpc64-unknown-aix -emit-llvm %s -o - | \
+// RUN: %clang_cc1 -O2 -triple=powerpc64-unknown-aix -emit-llvm %s -

[PATCH] D105754: [PowerPC] Fix L[D|W]ARX Implementation

[Albion Fung via Phabricator via cfe-commits]

Conanap updated this revision to Diff 358032.
Conanap added a comment.

Updated lwarx test case with modifier


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105754

Files:
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/test/CodeGen/builtins-ppc-xlcompat-LoadReseve-StoreCond-64bit-only.c
  clang/test/CodeGen/builtins-ppc-xlcompat-LoadReseve-StoreCond.c
  llvm/include/llvm/IR/IntrinsicsPowerPC.td
  llvm/lib/Target/PowerPC/PPCInstr64Bit.td
  llvm/lib/Target/PowerPC/PPCInstrInfo.td
  
llvm/test/CodeGen/PowerPC/builtins-ppc-xlcompat-LoadReserve-StoreCond-64bit-only.ll
  llvm/test/CodeGen/PowerPC/builtins-ppc-xlcompat-LoadReserve-StoreCond.ll

Index: llvm/test/CodeGen/PowerPC/builtins-ppc-xlcompat-LoadReserve-StoreCond.ll
===
--- llvm/test/CodeGen/PowerPC/builtins-ppc-xlcompat-LoadReserve-StoreCond.ll
+++ llvm/test/CodeGen/PowerPC/builtins-ppc-xlcompat-LoadReserve-StoreCond.ll
@@ -12,18 +12,21 @@
 define dso_local signext i32 @test_lwarx(i32* readnone %a) {
 ; CHECK-64-LABEL: test_lwarx:
 ; CHECK-64:   # %bb.0: # %entry
+; CHECK-64-NEXT:#APP
 ; CHECK-64-NEXT:lwarx 3, 0, 3
+; CHECK-64-NEXT:#NO_APP
 ; CHECK-64-NEXT:extsw 3, 3
 ; CHECK-64-NEXT:blr
 ;
 ; CHECK-32-LABEL: test_lwarx:
 ; CHECK-32:   # %bb.0: # %entry
+; CHECK-32-NEXT:#APP
 ; CHECK-32-NEXT:lwarx 3, 0, 3
+; CHECK-32-NEXT:#NO_APP
 ; CHECK-32-NEXT:blr
 entry:
-  %0 = bitcast i32* %a to i8*
-  %1 = tail call i32 @llvm.ppc.lwarx(i8* %0)
-  ret i32 %1
+  %0 = call i32 asm sideeffect "lwarx $0, ${1:y}", "=r,*Z,~{memory}"(i32* %a)
+  ret i32 %0
 }
 
 declare i32 @llvm.ppc.stwcx(i8*, i32)
Index: llvm/test/CodeGen/PowerPC/builtins-ppc-xlcompat-LoadReserve-StoreCond-64bit-only.ll
===
--- llvm/test/CodeGen/PowerPC/builtins-ppc-xlcompat-LoadReserve-StoreCond-64bit-only.ll
+++ llvm/test/CodeGen/PowerPC/builtins-ppc-xlcompat-LoadReserve-StoreCond-64bit-only.ll
@@ -10,17 +10,18 @@
 define dso_local i64 @test_ldarx(i64* readnone %a) {
 ; CHECK-LABEL: test_ldarx:
 ; CHECK:   # %bb.0: # %entry
+; CHECK-NEXT:#APP
 ; CHECK-NEXT:ldarx 3, 0, 3
+; CHECK-NEXT:#NO_APP
 ; CHECK-NEXT:blr
 entry:
-  %0 = bitcast i64* %a to i8*
-  %1 = tail call i64 @llvm.ppc.ldarx(i8* %0)
-  ret i64 %1
+  %0 = call i64 asm sideeffect "ldarx $0, ${1:y}", "=r,*Z,~{memory}"(i64* %a)
+  ret i64 %0
 }
 
 declare i32 @llvm.ppc.stdcx(i8*, i64)
-define dso_local i64 @test(i64* %a, i64 %b) {
-; CHECK-LABEL: test:
+define dso_local i64 @test_stdcx(i64* %a, i64 %b) {
+; CHECK-LABEL: test_stdcx:
 ; CHECK:   # %bb.0: # %entry
 ; CHECK-NEXT:stdcx. 4, 0, 3
 ; CHECK-NEXT:mfocrf 3, 128
Index: llvm/lib/Target/PowerPC/PPCInstrInfo.td
===
--- llvm/lib/Target/PowerPC/PPCInstrInfo.td
+++ llvm/lib/Target/PowerPC/PPCInstrInfo.td
@@ -5412,7 +5412,5 @@
 def : Pat<(i64 (bitreverse i64:$A)),
   (OR8 (RLDICR DWBytes7654.DWord, 32, 31), DWBytes3210.DWord)>;
 
-def : Pat<(int_ppc_lwarx ForceXForm:$dst),
-  (LWARX ForceXForm:$dst)>;
 def : Pat<(int_ppc_stwcx ForceXForm:$dst, gprc:$A),
   (STWCX gprc:$A, ForceXForm:$dst)>;
Index: llvm/lib/Target/PowerPC/PPCInstr64Bit.td
===
--- llvm/lib/Target/PowerPC/PPCInstr64Bit.td
+++ llvm/lib/Target/PowerPC/PPCInstr64Bit.td
@@ -1723,5 +1723,3 @@
 
 def : Pat<(int_ppc_stdcx ForceXForm:$dst, g8rc:$A),
   (STDCX g8rc:$A, ForceXForm:$dst)>;
-def : Pat<(int_ppc_ldarx ForceXForm:$dst),
-  (LDARX ForceXForm:$dst)>;
Index: llvm/include/llvm/IR/IntrinsicsPowerPC.td
===
--- llvm/include/llvm/IR/IntrinsicsPowerPC.td
+++ llvm/include/llvm/IR/IntrinsicsPowerPC.td
@@ -1529,9 +1529,5 @@
   def int_ppc_stwcx : GCCBuiltin<"__builtin_ppc_stwcx">,
   Intrinsic<[llvm_i32_ty], [llvm_ptr_ty, llvm_i32_ty],
 [IntrWriteMem]>;
-  def int_ppc_lwarx : GCCBuiltin<"__builtin_ppc_lwarx">,
-  Intrinsic<[llvm_i32_ty], [llvm_ptr_ty], [IntrNoMem]>;
-  def int_ppc_ldarx : GCCBuiltin<"__builtin_ppc_ldarx">,
-  Intrinsic<[llvm_i64_ty], [llvm_ptr_ty], [IntrNoMem]>;
 }
 
Index: clang/test/CodeGen/builtins-ppc-xlcompat-LoadReseve-StoreCond.c
===
--- clang/test/CodeGen/builtins-ppc-xlcompat-LoadReseve-StoreCond.c
+++ clang/test/CodeGen/builtins-ppc-xlcompat-LoadReseve-StoreCond.c
@@ -1,22 +1,20 @@
-// RUN: %clang_cc1 -triple=powerpc-unknown-aix -emit-llvm %s -o - | \
+// RUN: %clang_cc1 -O2 -triple=powerpc-unknown-aix -emit-llvm %s -o - | \
 // RUN: FileCheck %s
-// RUN: %clang_cc1 -triple=powerpc64-unknown-aix -emit-llvm %s -o - | \
+// RUN: %clang_cc1 -O2 -triple=powerpc64-unkno

[PATCH] D105765: Prepare Compiler-RT for GnuInstallDirs, matching libcxx, document all

[Petr Hosek via Phabricator via cfe-commits]

phosek added inline comments.



Comment at: compiler-rt/docs/BuildingCompilerRT.rst:12-22
+The instructions on this page are aimed at vendors who ship Compiler-RT as 
part of an
+operating system distribution, a toolchain or similar shipping vehicules. If 
you
+are a user merely trying to use Compiler-RT in your program, you most likely 
want to
+refer to your vendor's documentation, or to the general documentation for using
+Compiler-RT :ref:`here `.
+
+.. warning::

Ericson2314 wrote:
> Modeled this file off libc++'s, including taking these two paragraphs. They 
> seem appropriate enough here.
I'm not sure how appropriate this is actually since Compiler-RT is usually 
shipped together with the compiler, not the operating system (unlike libc++) 
since the LLVM and Compiler-RT ABI is tightly coupled. I'd omit this for now.



Comment at: compiler-rt/docs/BuildingCompilerRT.rst:16
+refer to your vendor's documentation, or to the general documentation for using
+Compiler-RT :ref:`here `.
+

compiler-rt?



Comment at: compiler-rt/docs/BuildingCompilerRT.rst:59
+  ``-DCOMPILER_RT_INSTALL_PATH:PATH=...`` not
+  ``-DCOMPILER_RT_INSTALL_PATH:PATH=...``, otherwise CMake will convert the
+  path to an absolute path.

Is this what you meant here?



Comment at: compiler-rt/docs/BuildingCompilerRT.rst:87
+
+  Path where Compiler-RT headers should be installed. If a relative
+  path, relative to ``COMPILER_RT_INSTALL_PATH``.

This variable is used for data files (like resources), not headers.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105765

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

[PATCH] D105765: Prepare Compiler-RT for GnuInstallDirs, matching libcxx, document all

[John Ericson via Phabricator via cfe-commits]

Ericson2314 updated this revision to Diff 358057.
Ericson2314 marked 2 inline comments as done.
Ericson2314 added a comment.

Fixes from comments, Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105765

Files:
  clang/runtime/CMakeLists.txt
  compiler-rt/cmake/Modules/AddCompilerRT.cmake
  compiler-rt/cmake/Modules/CompilerRTDarwinUtils.cmake
  compiler-rt/cmake/Modules/CompilerRTUtils.cmake
  compiler-rt/cmake/base-config-ix.cmake
  compiler-rt/docs/BuildingCompilerRT.rst
  compiler-rt/include/CMakeLists.txt
  compiler-rt/lib/dfsan/CMakeLists.txt
  libcxx/CMakeLists.txt
  libcxx/docs/BuildingLibcxx.rst
  libcxxabi/CMakeLists.txt
  libunwind/CMakeLists.txt
  libunwind/docs/BuildingLibunwind.rst

Index: libunwind/docs/BuildingLibunwind.rst
===
--- libunwind/docs/BuildingLibunwind.rst
+++ libunwind/docs/BuildingLibunwind.rst
@@ -159,3 +159,10 @@
 .. option:: LIBUNWIND_SYSROOT
 
   Sysroot for cross compiling
+
+.. option:: LIBUNWIND_INSTALL_LIBRARY_DIR:PATH
+
+  **Default**: ``lib${LIBUNWIND_LIBDIR_SUFFIX}``
+
+  Path where built libunwind libraries should be installed. If a relative path,
+  relative to ``CMAKE_INSTALL_PREFIX``.
Index: libunwind/CMakeLists.txt
===
--- libunwind/CMakeLists.txt
+++ libunwind/CMakeLists.txt
@@ -116,17 +116,20 @@
 
 if(LLVM_ENABLE_PER_TARGET_RUNTIME_DIR AND NOT APPLE)
   set(LIBUNWIND_LIBRARY_DIR ${LLVM_LIBRARY_OUTPUT_INTDIR}/${LLVM_DEFAULT_TARGET_TRIPLE})
-  set(LIBUNWIND_INSTALL_LIBRARY_DIR lib${LLVM_LIBDIR_SUFFIX}/${LLVM_DEFAULT_TARGET_TRIPLE})
+  set(LIBUNWIND_INSTALL_LIBRARY_DIR lib${LLVM_LIBDIR_SUFFIX}/${LLVM_DEFAULT_TARGET_TRIPLE} CACHE PATH
+  "Path where built libunwind libraries should be installed.")
   if(LIBCXX_LIBDIR_SUBDIR)
 string(APPEND LIBUNWIND_LIBRARY_DIR /${LIBUNWIND_LIBDIR_SUBDIR})
 string(APPEND LIBUNWIND_INSTALL_LIBRARY_DIR /${LIBUNWIND_LIBDIR_SUBDIR})
   endif()
 elseif(LLVM_LIBRARY_OUTPUT_INTDIR)
   set(LIBUNWIND_LIBRARY_DIR ${LLVM_LIBRARY_OUTPUT_INTDIR})
-  set(LIBUNWIND_INSTALL_LIBRARY_DIR lib${LIBUNWIND_LIBDIR_SUFFIX})
+  set(LIBUNWIND_INSTALL_LIBRARY_DIR lib${LIBUNWIND_LIBDIR_SUFFIX} CACHE PATH
+  "Path where built libunwind libraries should be installed.")
 else()
   set(LIBUNWIND_LIBRARY_DIR ${CMAKE_BINARY_DIR}/lib${LIBUNWIND_LIBDIR_SUFFIX})
-  set(LIBUNWIND_INSTALL_LIBRARY_DIR lib${LIBUNWIND_LIBDIR_SUFFIX})
+  set(LIBUNWIND_INSTALL_LIBRARY_DIR lib${LIBUNWIND_LIBDIR_SUFFIX} CACHE PATH
+  "Path where built libunwind libraries should be installed.")
 endif()
 
 set(CMAKE_ARCHIVE_OUTPUT_DIRECTORY ${LIBUNWIND_LIBRARY_DIR})
Index: libcxxabi/CMakeLists.txt
===
--- libcxxabi/CMakeLists.txt
+++ libcxxabi/CMakeLists.txt
@@ -195,7 +195,8 @@
 if(LLVM_ENABLE_PER_TARGET_RUNTIME_DIR AND NOT APPLE)
   set(LIBCXXABI_HEADER_DIR ${LLVM_BINARY_DIR})
   set(LIBCXXABI_LIBRARY_DIR ${LLVM_LIBRARY_OUTPUT_INTDIR}/${LLVM_DEFAULT_TARGET_TRIPLE})
-  set(LIBCXXABI_INSTALL_LIBRARY_DIR lib${LLVM_LIBDIR_SUFFIX}/${LLVM_DEFAULT_TARGET_TRIPLE})
+  set(LIBCXXABI_INSTALL_LIBRARY_DIR lib${LLVM_LIBDIR_SUFFIX}/${LLVM_DEFAULT_TARGET_TRIPLE} CACHE PATH
+  "Path where built libc++abi libraries should be installed.")
   if(LIBCXX_LIBDIR_SUBDIR)
 string(APPEND LIBCXXABI_LIBRARY_DIR /${LIBCXXABI_LIBDIR_SUBDIR})
 string(APPEND LIBCXXABI_INSTALL_LIBRARY_DIR /${LIBCXXABI_LIBDIR_SUBDIR})
@@ -203,11 +204,13 @@
 elseif(LLVM_LIBRARY_OUTPUT_INTDIR)
   set(LIBCXXABI_HEADER_DIR ${LLVM_BINARY_DIR})
   set(LIBCXXABI_LIBRARY_DIR ${LLVM_LIBRARY_OUTPUT_INTDIR})
-  set(LIBCXXABI_INSTALL_LIBRARY_DIR lib${LIBCXXABI_LIBDIR_SUFFIX})
+  set(LIBCXXABI_INSTALL_LIBRARY_DIR lib${LIBCXXABI_LIBDIR_SUFFIX} CACHE PATH
+  "Path where built libc++abi libraries should be installed.")
 else()
   set(LIBCXXABI_HEADER_DIR ${CMAKE_BINARY_DIR})
   set(LIBCXXABI_LIBRARY_DIR ${CMAKE_BINARY_DIR}/lib${LIBCXXABI_LIBDIR_SUFFIX})
-  set(LIBCXXABI_INSTALL_LIBRARY_DIR lib${LIBCXXABI_LIBDIR_SUFFIX})
+  set(LIBCXXABI_INSTALL_LIBRARY_DIR lib${LIBCXXABI_LIBDIR_SUFFIX} CACHE PATH
+  "Path where built libc++abi libraries should be installed.")
 endif()
 
 set(CMAKE_ARCHIVE_OUTPUT_DIRECTORY ${LIBCXXABI_LIBRARY_DIR})
Index: libcxx/docs/BuildingLibcxx.rst
===
--- libcxx/docs/BuildingLibcxx.rst
+++ libcxx/docs/BuildingLibcxx.rst
@@ -251,6 +251,28 @@
This option can be used to enable or disable the filesystem components on
platforms that may not support them. For example on Windows.
 
+.. option:: LIBCXX_INSTALL_LIBRARY_DIR:PATH
+
+  **Default**: ``lib${LIBCXX_LIBDIR_SUFFIX}``
+
+  Path where built libc++ libraries should be installed. If a relative path,
+  relative to ``CMAKE_INSTALL_PREFIX``.
+
+.. option:: LIBCXX_INSTALL_INCLUDE_DIR:PATH
+
+  **Default*

[PATCH] D105765: Prepare Compiler-RT for GnuInstallDirs, matching libcxx, document all

[John Ericson via Phabricator via cfe-commits]

Ericson2314 added inline comments.



Comment at: compiler-rt/docs/BuildingCompilerRT.rst:12-22
+The instructions on this page are aimed at vendors who ship Compiler-RT as 
part of an
+operating system distribution, a toolchain or similar shipping vehicules. If 
you
+are a user merely trying to use Compiler-RT in your program, you most likely 
want to
+refer to your vendor's documentation, or to the general documentation for using
+Compiler-RT :ref:`here `.
+
+.. warning::

phosek wrote:
> Ericson2314 wrote:
> > Modeled this file off libc++'s, including taking these two paragraphs. They 
> > seem appropriate enough here.
> I'm not sure how appropriate this is actually since Compiler-RT is usually 
> shipped together with the compiler, not the operating system (unlike libc++) 
> since the LLVM and Compiler-RT ABI is tightly coupled. I'd omit this for now.
Fair enough. Removed.



Comment at: compiler-rt/docs/BuildingCompilerRT.rst:16
+refer to your vendor's documentation, or to the general documentation for using
+Compiler-RT :ref:`here `.
+

phosek wrote:
> compiler-rt?
Ah there is no such section for `compiler-rt/docs`. I deleted and reworded.



Comment at: compiler-rt/docs/BuildingCompilerRT.rst:59
+  ``-DCOMPILER_RT_INSTALL_PATH:PATH=...`` not
+  ``-DCOMPILER_RT_INSTALL_PATH:PATH=...``, otherwise CMake will convert the
+  path to an absolute path.

phosek wrote:
> Is this what you meant here?
Oops. Yes.



Comment at: compiler-rt/docs/BuildingCompilerRT.rst:87
+
+  Path where Compiler-RT headers should be installed. If a relative
+  path, relative to ``COMPILER_RT_INSTALL_PATH``.

phosek wrote:
> This variable is used for data files (like resources), not headers.
Says "data" like the CMake variale description does now too.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105765

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

[PATCH] D105754: [PowerPC] Fix L[D|W]ARX Implementation

[Nemanja Ivanovic via Phabricator via cfe-commits]

nemanjai accepted this revision.
nemanjai 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/D105754/new/

https://reviews.llvm.org/D105754

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

[PATCH] D105765: Prepare Compiler-RT for GnuInstallDirs, matching libcxx, document all

[Petr Hosek via Phabricator via cfe-commits]

phosek accepted this revision.
phosek added a comment.

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105765

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

[PATCH] D69764: [clang-format] Add East/West Const fixer capability

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D69764#2873225 , @MyDeveloperDay 
wrote:

> If this is something we think we wouldn't want in clang-format, what about 
> the idea of building a new binary which did allow the modification of the 
> tokens, This comes up from time to time and I kind of feel its a weak 
> argument for those that don't want to use it enforcing their will on those 
> that do.
>
> I feel we have D69764: [clang-format] Add East/West Const fixer capability 
>  and D95168: [clang-format] Add InsertBraces 
> option  both of which seem to break the 
> fundamental belief that some feel that we shouldn't change anything other 
> than whitespace (A fact that has already been broken 3 times already).
>
> But really these are just separate passes, so why couldn't clang-format allow 
> custom passes to be added much like any compiler does.
>
> If the objection is that this isn't clang-format's role then what if we build 
> something new whose purpose IS to allow these kinds of changes?

What you're describing sounds like clang-tidy but perhaps with improved support 
for composing source modifications from fix-its, or do you envision something 
rather different?


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

https://reviews.llvm.org/D69764

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

[PATCH] D105881: [flang][driver] Switch to `BoolFOption` for boolean options

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 added inline comments.



Comment at: clang/include/clang/Driver/Options.td:246
 
+class EmptyKPM : KeyPathAndMacro<"", "", ""> {}
 class DiagnosticOpts

awarzynski wrote:
> @jansvoboda11 , is this the right approach here? I'd like use `BoolFOption` 
> in Flang, but we don't need `KeyPathAndMacro` just yet. We may do in the 
> future.
There was an `EmptyKPM` class present in the code before, but only as an 
implementation detail of the marshalling system. People started using it with 
`BoolFOption` to conveniently declare pairs Clang driver flags. I'm not a fan 
since the whole `BooFOption` is designed around the keypath and its 
marshalling. Putting a "null" keypath in `BoolFOption` and then talking about 
its defaults and the effect of each flag seems highly unintuitive to me and 
it's essentially dead code. I'd prefer a different approach in this patch 
unless you have plans to switch to the marshalling infrastructure and remove 
`EmptyKPM` in the short term.

Would something similar to `OptInFFlag` and `OptOutFFlag` work for you? You 
could generalize it (remove `Flags<[CC1Option]>`), make use of it for Flang's 
options directly and forward the current `Opt{In,Out}FFlag` to the 
generalization while specifying the `CC1Option`. WDYT?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105881

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

[PATCH] D103191: [OpenCL] Add support of __opencl_c_program_scope_global_variables feature macro

[Anton Zabaznov via Phabricator via cfe-commits]

azabaznov updated this revision to Diff 358236.
azabaznov added a comment.

Update test after generic AS was merged


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103191

Files:
  clang/include/clang/Basic/OpenCLOptions.h
  clang/lib/Sema/SemaDecl.cpp
  clang/test/CodeGenOpenCL/addr-space-struct-arg.cl
  clang/test/SemaOpenCL/storageclass.cl

Index: clang/test/SemaOpenCL/storageclass.cl
===
--- clang/test/SemaOpenCL/storageclass.cl
+++ clang/test/SemaOpenCL/storageclass.cl
@@ -1,28 +1,115 @@
 // RUN: %clang_cc1 %s -verify -pedantic -fsyntax-only -cl-std=CL1.2
-
+// RUN: %clang_cc1 %s -verify -pedantic -fsyntax-only -cl-std=CL3.0 -cl-ext=-__opencl_c_program_scope_global_variables,-__opencl_c_generic_address_space
+// RUN: %clang_cc1 %s -verify -pedantic -fsyntax-only -cl-std=CL3.0 -cl-ext=+__opencl_c_program_scope_global_variables,-__opencl_c_generic_address_space
+// RUN: %clang_cc1 %s -verify -pedantic -fsyntax-only -cl-std=CL3.0 -cl-ext=-__opencl_c_program_scope_global_variables,+__opencl_c_generic_address_space
+// RUN: %clang_cc1 %s -verify -pedantic -fsyntax-only -cl-std=CL3.0 -cl-ext=+__opencl_c_program_scope_global_variables,+__opencl_c_generic_address_space
 static constant int G1 = 0;
 constant int G2 = 0;
-int G3 = 0;// expected-error{{program scope variable must reside in constant address space}}
-global int G4 = 0; // expected-error{{program scope variable must reside in constant address space}}
 
-static float g_implicit_static_var = 0; // expected-error {{program scope variable must reside in constant address space}}
+int G3 = 0;
+#ifndef __opencl_c_program_scope_global_variables
+// expected-error@-2 {{program scope variable must reside in constant address space}}
+#endif
+
+global int G4 = 0;
+#ifndef __opencl_c_program_scope_global_variables
+// expected-error@-2 {{program scope variable must reside in constant address space}}
+#endif
+
+static float g_implicit_static_var = 0;
+#ifndef __opencl_c_program_scope_global_variables
+// expected-error@-2 {{program scope variable must reside in constant address space}}
+#endif
+
 static constant float g_constant_static_var = 0;
-static global float g_global_static_var = 0;   // expected-error {{program scope variable must reside in constant address space}}
-static local float g_local_static_var = 0; // expected-error {{program scope variable must reside in constant address space}}
-static private float g_private_static_var = 0; // expected-error {{program scope variable must reside in constant address space}}
-static generic float g_generic_static_var = 0; // expected-error{{OpenCL C version 1.2 does not support the 'generic' type qualifier}} // expected-error {{program scope variable must reside in constant address space}}
 
-extern float g_implicit_extern_var; // expected-error {{extern variable must reside in constant address space}}
+static global float g_global_static_var = 0;
+#ifndef __opencl_c_program_scope_global_variables
+// expected-error@-2 {{program scope variable must reside in constant address space}}
+#endif
+
+static local float g_local_static_var = 0;
+#ifndef __opencl_c_program_scope_global_variables
+// expected-error@-2 {{program scope variable must reside in constant address space}}
+#else
+// expected-error@-4 {{program scope variable must reside in global or constant address space}}
+#endif
+
+static private float g_private_static_var = 0;
+#ifndef __opencl_c_program_scope_global_variables
+// expected-error@-2 {{program scope variable must reside in constant address space}}
+#else
+// expected-error@-4 {{program scope variable must reside in global or constant address space}}
+#endif
+
+static generic float g_generic_static_var = 0;
+#if (__OPENCL_C_VERSION__ < 300)
+// expected-error@-2 {{OpenCL C version 1.2 does not support the 'generic' type qualifier}}
+// expected-error@-3 {{program scope variable must reside in constant address space}}
+#elif (__OPENCL_C_VERSION__ == 300)
+ #if !defined(__opencl_c_generic_address_space)
+// expected-error@-6 {{OpenCL C version 3.0 does not support the 'generic' type qualifier}}
+ #endif
+ #if !defined(__opencl_c_program_scope_global_variables)
+// expected-error@-9 {{program scope variable must reside in constant address space}}
+ #endif
+ #if defined(__opencl_c_generic_address_space) && defined(__opencl_c_program_scope_global_variables)
+// expected-error@-12 {{program scope variable must reside in global or constant address space}}
+ #endif
+#endif
+
+extern float g_implicit_extern_var;
+#ifndef __opencl_c_program_scope_global_variables
+// expected-error@-2 {{extern variable must reside in constant address space}}
+#endif
+
 extern constant float g_constant_extern_var;
-extern global float g_global_extern_var;   // expected-error {{extern variable must reside in constant address space}}
-extern local float g_local_ext

[PATCH] D105478: [clang] Make CXXRecrdDecl invalid if it contains any undeduced fields.

[Haojian Wu via Phabricator via cfe-commits]

hokein added a comment.

In D105478#2867120 , @adamcz wrote:

> OK, I think we've got it now :-)
>
> I kept the original crash test in the change, but I'm not sure if it's 
> appropriate anymore. Let me know if you think I should remove it.

Thanks, that sounds good to me, the original crash test is valuable, worth to 
keep it.




Comment at: clang/lib/Parse/ParseDeclCXX.cpp:2987
+if (ThisDecl)
+  ThisDecl->setInvalidDecl();
 SkipUntil(tok::comma, StopAtSemi | StopBeforeMatch);

it seems to me calling `Sema::ActOnInitializerError(ThisDecl)` is a better fit, 
rather than invalidating the decl once the initializer is failed to parse, for 
example `static const int Member = ;` we want to keep the decl valid.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105478

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

[PATCH] D105014: added some example code for llvm::Expected

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

In D105014#2872674 , @dblaikie wrote:

> +@lhames for context as the author/owner of `llvm::Error` and associated 
> things.
>
> Perhaps it'd be handy to have some descriptions of the problems you 
> encountered, @kuhnel, and how you went about trying to resolve them, to help 
> understand where things are lacking.
>
>> @sammccall 
>> I agree Error is confusing and better docs might help.
>> (Not as much as removing the confusing part of the design, but I that seems 
>> hard at this point!)
>
> Maybe in this review, or maybe elsewhere: might be good to have some details 
> on your perspective here?

Mostly that the attempt to enforce handling is an interesting idea but a cure 
worse than the disease in practice. In interacting closely with ~6 developers 
over the last ~4 years:

- most people were initially confused by this class and took significant time 
to build up a mental model, and are still fuzzy (e.g. on how it interacts with 
move semantics). I think this was Christian's experience here.
- In practice, developers who know Error well still sometimes check in code 
with "unhandled" errors by mistake
- most such code is correct except for the enforcement itself. e.g. test code 
that's missing a `cantFail(...)`, production code missing a 
`consumeError(Exp.takeError())` after handling failure.
- In practice, these mistake are difficult for reviewers who know Error well to 
catch
- it causes the Error class to have interface warts (`toString()` consumes the 
move-only object, operator== is non-const) that cause surprising ergonomic 
problems, e.g. requiring special helpers when used from test code

We've talked about the possibility of adding separate Error/Expected types to 
clangd with similar semantics but no check-enforcement and a simpler payload 
model. However having to deal with both error types seems too painful.

There's one case that's problematic enough to cause real danger, and has bitten 
us a couple of times:

- an `Expected` is obtained and checked for success
- dynamically the value is almost always OK (e.g. except when out of disk space)
- in the failure case, the error is handled but not consumed. This is not 
caught in tests because the error condition is hard to simulated
- now months later when the error occurs in the wild in a debug build (yes, 
people seem to use these...) it becomes a crash

---

Separately from this issue, creating/logging errors is pretty clunky in the 
common case, and error handling is ubiquitous.
I wonder if there's appetite for the `error()` function from 
`clang-tools-extra/clangd/support/Logger.h` to live somewhere common.
(Both `error()` and the support for errors in `log()` in that file are pretty 
useful quality-of-life features, but `log()` is maybe not well-suited for llvm)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105014

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

[clang] ab76101 - [OpenCL] Add support of __opencl_c_read_write_images feature macro

[Anton Zabaznov via cfe-commits]

Author: Anton Zabaznov
Date: 2021-07-13T15:38:23+03:00
New Revision: ab76101f40f80bbec82073fc5bfddd7203e63a52

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

LOG: [OpenCL] Add support of __opencl_c_read_write_images feature macro

This feature requires support of __opencl_c_images, so diagnostics for that is 
provided as well

Reviewed By: Anastasia

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

Added: 


Modified: 
clang/include/clang/Basic/DiagnosticCommonKinds.td
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/include/clang/Basic/OpenCLOptions.h
clang/lib/Basic/OpenCLOptions.cpp
clang/lib/Basic/Targets.cpp
clang/lib/Sema/SemaDeclAttr.cpp
clang/test/Misc/opencl-c-3.0.incorrect_options.cl
clang/test/SemaOpenCL/access-qualifier.cl
clang/test/SemaOpenCL/unsupported-image.cl

Removed: 




diff  --git a/clang/include/clang/Basic/DiagnosticCommonKinds.td 
b/clang/include/clang/Basic/DiagnosticCommonKinds.td
index 4dab3a3b39ac3..eea5d8eaa10a1 100644
--- a/clang/include/clang/Basic/DiagnosticCommonKinds.td
+++ b/clang/include/clang/Basic/DiagnosticCommonKinds.td
@@ -367,4 +367,6 @@ def warn_opencl_unsupported_core_feature : Warning<
 
 def err_opencl_extension_and_feature_
diff ers : Error<
   "options %0 and %1 are set to 
diff erent values">;
+def err_opencl_feature_requires : Error<
+  "feature %0 requires support of %1 feature">;
 }

diff  --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td 
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 21b9ce2d9ff2a..2d62163e3dcc0 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -10111,7 +10111,8 @@ def err_opencl_builtin_pipe_invalid_access_modifier : 
Error<
 def err_opencl_invalid_access_qualifier : Error<
   "access qualifier can only be used for pipe and image type">;
 def err_opencl_invalid_read_write : Error<
-  "access qualifier %0 can not be used for %1 %select{|prior to OpenCL version 
2.0}2">;
+  "access qualifier %0 can not be used for %1 %select{|prior to OpenCL C 
version 2.0 or in version 3.0 "
+  "and without __opencl_c_read_write_images feature}2">;
 def err_opencl_multiple_access_qualifiers : Error<
   "multiple access qualifiers">;
 def note_opencl_typedef_access_qualifier : Note<

diff  --git a/clang/include/clang/Basic/OpenCLOptions.h 
b/clang/include/clang/Basic/OpenCLOptions.h
index 44fda029411f1..41db6b712a631 100644
--- a/clang/include/clang/Basic/OpenCLOptions.h
+++ b/clang/include/clang/Basic/OpenCLOptions.h
@@ -19,6 +19,9 @@
 
 namespace clang {
 
+class DiagnosticsEngine;
+class TargetInfo;
+
 namespace {
 // This enum maps OpenCL version(s) into value. These values are used as
 // a mask to indicate in which OpenCL version(s) extension is a core or
@@ -179,6 +182,16 @@ class OpenCLOptions {
 return OpenCLOptionInfo(std::forward(args)...).isAvailableIn(LO);
   }
 
+  // Diagnose feature dependencies for OpenCL C 3.0. Return false if target
+  // doesn't follow these requirements.
+  static bool diagnoseUnsupportedFeatureDependencies(const TargetInfo &TI,
+ DiagnosticsEngine &Diags);
+
+  // Diagnose that features and equivalent extension are set to same values.
+  // Return false if target doesn't follow these requirements.
+  static bool diagnoseFeatureExtensionDifferences(const TargetInfo &TI,
+  DiagnosticsEngine &Diags);
+
 private:
   // Option is enabled via pragma
   bool isEnabled(llvm::StringRef Ext) const;

diff  --git a/clang/lib/Basic/OpenCLOptions.cpp 
b/clang/lib/Basic/OpenCLOptions.cpp
index 7a647f02b1bd0..2e215b185f662 100644
--- a/clang/lib/Basic/OpenCLOptions.cpp
+++ b/clang/lib/Basic/OpenCLOptions.cpp
@@ -7,6 +7,8 @@
 
//===--===//
 
 #include "clang/Basic/OpenCLOptions.h"
+#include "clang/Basic/Diagnostic.h"
+#include "clang/Basic/TargetInfo.h"
 
 namespace clang {
 
@@ -104,4 +106,43 @@ void OpenCLOptions::disableAll() {
 Opt.getValue().Enabled = false;
 }
 
+bool OpenCLOptions::diagnoseUnsupportedFeatureDependencies(
+const TargetInfo &TI, DiagnosticsEngine &Diags) {
+  // Feature pairs. First feature in a pair requires the second one to be
+  // supported.
+  static const llvm::StringMap DependentFeaturesMap = {
+  {"__opencl_c_read_write_images", "__opencl_c_images"}};
+
+  auto OpenCLFeaturesMap = TI.getSupportedOpenCLOpts();
+
+  bool IsValid = true;
+  for (auto &FeaturePair : DependentFeaturesMap)
+if (TI.hasFeatureEnabled(OpenCLFeaturesMap, FeaturePair.getKey()) &&
+!TI.hasFeatureEnabled(OpenCLFeaturesMap, FeaturePair.getValue())) {
+  IsValid = false;
+ 

[PATCH] D104915: [OpenCL] Add support of __opencl_c_read_write_images feature macro

[Anton Zabaznov via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGab76101f40f8: [OpenCL] Add support of 
__opencl_c_read_write_images feature macro (authored by azabaznov).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104915

Files:
  clang/include/clang/Basic/DiagnosticCommonKinds.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Basic/OpenCLOptions.h
  clang/lib/Basic/OpenCLOptions.cpp
  clang/lib/Basic/Targets.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/Misc/opencl-c-3.0.incorrect_options.cl
  clang/test/SemaOpenCL/access-qualifier.cl
  clang/test/SemaOpenCL/unsupported-image.cl

Index: clang/test/SemaOpenCL/unsupported-image.cl
===
--- clang/test/SemaOpenCL/unsupported-image.cl
+++ clang/test/SemaOpenCL/unsupported-image.cl
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -triple spir-unknown-unknown -verify -cl-std=CL3.0 -cl-ext=-__opencl_c_images %s
+// RUN: %clang_cc1 -triple spir-unknown-unknown -verify -cl-std=CL3.0 -cl-ext=-__opencl_c_images,-__opencl_c_read_write_images %s
 // RUN: %clang_cc1 -triple spir-unknown-unknown -verify -cl-std=CL3.0 -cl-ext=+__opencl_c_images %s
 
 #ifdef __opencl_c_images
Index: clang/test/SemaOpenCL/access-qualifier.cl
===
--- clang/test/SemaOpenCL/access-qualifier.cl
+++ clang/test/SemaOpenCL/access-qualifier.cl
@@ -1,12 +1,14 @@
-// RUN: %clang_cc1 -verify -pedantic -fsyntax-only -cl-std=CL1.2 %s -cl-ext=-cl_khr_3d_image_writes
-// RUN: %clang_cc1 -verify -pedantic -fsyntax-only -cl-std=CL2.0 %s
+// RUN: %clang_cc1 -triple spir-unknown-unknown -verify -pedantic -fsyntax-only -cl-std=CL1.2 %s -cl-ext=-cl_khr_3d_image_writes
+// RUN: %clang_cc1 -triple spir-unknown-unknown -verify -pedantic -fsyntax-only -cl-std=CL2.0 %s
+// RUN: %clang_cc1 -triple spir-unknown-unknown -verify -pedantic -fsyntax-only -cl-std=CL3.0 %s
+// RUN: %clang_cc1 -triple spir-unknown-unknown -verify -pedantic -fsyntax-only -cl-std=CL3.0 %s -cl-ext=-__opencl_c_read_write_images
 
 typedef image1d_t img1d_ro_default; // expected-note {{previously declared 'read_only' here}}
 
 typedef write_only image1d_t img1d_wo; // expected-note {{previously declared 'write_only' here}}
 typedef read_only image1d_t img1d_ro;
 
-#if __OPENCL_C_VERSION__ >= 200
+#if (__OPENCL_C_VERSION__ == 200) || (__OPENCL_C_VERSION__ == 300 && defined(__opencl_c_read_write_images))
 typedef read_write image1d_t img1d_rw;
 #endif
 
@@ -17,10 +19,10 @@
 void myWrite(write_only image1d_t); // expected-note {{passing argument to parameter here}} expected-note {{passing argument to parameter here}}
 void myRead(read_only image1d_t); // expected-note {{passing argument to parameter here}}
 
-#if __OPENCL_C_VERSION__ >= 200
+#if (__OPENCL_C_VERSION__ == 200) || (__OPENCL_C_VERSION__ == 300 && defined(__opencl_c_read_write_images))
 void myReadWrite(read_write image1d_t);
 #else
-void myReadWrite(read_write image1d_t); // expected-error {{access qualifier 'read_write' can not be used for '__read_write image1d_t' prior to OpenCL version 2.0}}
+void myReadWrite(read_write image1d_t); // expected-error {{access qualifier 'read_write' can not be used for '__read_write image1d_t' prior to OpenCL C version 2.0 or in version 3.0 and without __opencl_c_read_write_images feature}}
 #endif
 
 
@@ -36,7 +38,7 @@
   myWrite(img);
 }
 
-#if __OPENCL_C_VERSION__ >= 200
+#if (__OPENCL_C_VERSION__ == 200) || (__OPENCL_C_VERSION__ == 300 && defined(__opencl_c_read_write_images))
 kernel void k4(img1d_rw img) {
   myReadWrite(img);
 }
@@ -62,26 +64,26 @@
 
 kernel void k12(read_only read_only image1d_t i){} // expected-warning {{duplicate 'read_only' declaration specifier}}
 
-#if __OPENCL_C_VERSION__ >= 200
+#if (__OPENCL_C_VERSION__ == 200) || (__OPENCL_C_VERSION__ == 300 && defined(__opencl_c_read_write_images))
 kernel void k13(read_write pipe int i){} // expected-error{{access qualifier 'read_write' can not be used for 'read_only pipe int'}}
 #else
-kernel void k13(__read_write image1d_t i){} // expected-error{{access qualifier '__read_write' can not be used for '__read_write image1d_t' prior to OpenCL version 2.0}}
-#endif
-
-#if __OPENCL_C_VERSION__ >= 200
-void myPipeWrite(write_only pipe int); // expected-note {{passing argument to parameter here}}
-kernel void k14(read_only pipe int p) {
-  myPipeWrite(p); // expected-error {{passing '__private read_only pipe int' to parameter of incompatible type 'write_only pipe int'}}
-}
+kernel void k13(__read_write image1d_t i){} // expected-error{{access qualifier '__read_write' can not be used for '__read_write image1d_t' prior to OpenCL C version 2.0 or in version 3.0 and without __opencl_c_read_write_images feature}}
 #endif
 
 #if __OPENCL_C_VERSION__ < 200
 kernel void test_image3d_wo(write_only image3d_t img) {} // expected-error {{use o

[PATCH] D105679: [clangd] Add CMake option to (not) link in clang-tidy checks

[Sam McCall via Phabricator via cfe-commits]

sammccall marked 5 inline comments as done.
sammccall added a comment.

In D105679#2873109 , @nridge wrote:

> Will building with this option just prevent linking the clang-tidy checks 
> into clangd, or will it also prevent building them in the first place?

If you're running `ninja clangd` or `ninja check-clangd` they aren't built 
anymore even if dirty. (Just tested this)
This change breaks the only build dependency path between 
`clangd`/`ClangdTests` and the clang-tidy checks.

Of course `all`/`check-all`/`check-clang-tools` will still build the clang-tidy 
checks.




Comment at: clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp:280
 
+#if CLANGD_TIDY_CHECKS
 TEST_F(ConfigCompileTests, Tidy) {

kadircet wrote:
> why do we need to disable this test? it doesn't really build an ast or assert 
> on the diagnostics from clang-tidy in any way, seems to be purely about 
> configs.
We have clever logic that verifies that check names (but not wildcards) in 
configs actually match real checks.

Put ifdefs around the relevant assertions instead, so it should be now 
self-explanatory.



Comment at: clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp:259
 
+class T{$explicit[[]]$constructor[[T]](int a);};
+

kadircet wrote:
> ah, should've kept reading before leaving the comment above. feel free to 
> ignore that (or just drop this test and rely only on clang diagnostics for 
> checking that behaviour?)
This really belongs in the diagnostic ranges test, so switched to a different 
diag as you suggested.



Comment at: clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp:301
+  "macro 'SQUARE' defined here"))),
   Diag(Test.range("macroarg"),
+   "multiple unsequenced modifications to 'y'"),

kadircet wrote:
> i think we should suppress this with `-Wno-unsequenced` (and get rid of the 
> `Else` part). Moreover this is the only test requiring an Else bit (if I 
> didn't miss any). WDYT about just having a new file `TidyIntegrationTests` or 
> `TidyDiagnosticTests` and moving all of these there, or having two different 
> sections in this file to only enable these tests when tidy is on. It would 
> imply tests that want to check a mix of tidy and clang diagnostics needs to 
> go into the tidy section and cannot be tested on non-tidy builds, but that 
> sounds like the price we need to pay anyway, whereas introducing the 
> `ifTidyChecks` complicates things a little more (IMO).
> i think we should suppress this with -Wno-unsequenced (and get rid of the 
> Else part). Moreover this is the only test requiring an Else bit (if I didn't 
> miss any).

Done, and ripped out Else support.

> WDYT about just having a new file TidyIntegrationTests or TidyDiagnosticTests 
> and moving all of these there, or having two different sections in this file 
> to only enable these tests when tidy is on

I started with this but having substantial amounts of nontrivial code 
completely ifdef'd out makes me nervous about maintenance, as we'd lose all 
tooling support on these tests if we use this config.

This solution is significantly safer I think:
 - mostly preserves refactoring options even with clang-tidy off (e.g. local 
xrefs)
 - type checks all the matchers, even the clang-tidy ones with clang-tidy off
 - verifies that the code snippets in TestTU actually works
 - explicitly shows the difference with clang-tidy on vs off

> ifTidyChecks complicates things a little more (IMO).

Agree. I think the benefits of the tests being live code outweigh the 
relatively minor complexity. Maybe there's a better tradeoff to be had though.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105679

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

[PATCH] D105679: [clangd] Add CMake option to (not) link in clang-tidy checks

[Sam McCall via Phabricator via cfe-commits]

sammccall updated this revision to Diff 358241.
sammccall marked 3 inline comments as done.
sammccall added a comment.

Address review comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105679

Files:
  clang-tools-extra/clangd/CMakeLists.txt
  clang-tools-extra/clangd/Features.cpp
  clang-tools-extra/clangd/Features.inc.in
  clang-tools-extra/clangd/ParsedAST.cpp
  clang-tools-extra/clangd/test/diagnostics-tidy.test
  clang-tools-extra/clangd/test/diagnostics.test
  clang-tools-extra/clangd/test/lit.cfg.py
  clang-tools-extra/clangd/test/lit.site.cfg.py.in
  clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
  clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp

Index: clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
===
--- clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
+++ clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
@@ -10,6 +10,7 @@
 #include "Config.h"
 #include "Diagnostics.h"
 #include "FeatureModule.h"
+#include "Features.h"
 #include "ParsedAST.h"
 #include "Protocol.h"
 #include "SourceCode.h"
@@ -114,6 +115,18 @@
   return Res;
 }
 
+// Normally returns the provided diagnostics matcher.
+// If clang-tidy checks are not linked in, returns a matcher for no diagnostics!
+// This is intended for tests where the diagnostics come from clang-tidy checks.
+// We don't #ifdef each individual test as it's intrusive and we want to ensure
+// that as much of the test is still compiled an run as possible.
+::testing::Matcher>
+ifTidyChecks(::testing::Matcher> M) {
+  if (!CLANGD_TIDY_CHECKS)
+return IsEmpty();
+  return M;
+}
+
 TEST(DiagnosticsTest, DiagnosticRanges) {
   // Check we report correct ranges, including various edge-cases.
   Annotations Test(R"cpp(
@@ -121,8 +134,11 @@
 #define ID(X) X
 namespace test{};
 void $decl[[foo]]();
-class T{$explicit[[]]$constructor[[T]](int a);};
 int main() {
+  struct Container { int* begin(); int* end(); } *container;
+  for (auto i : $insertstar[[]]$range[[container]]) {
+  }
+
   $typo[[go\
 o]]();
   foo()$semicolon[[]]//with comments
@@ -135,10 +151,14 @@
 }
   )cpp");
   auto TU = TestTU::withCode(Test.code());
-  TU.ClangTidyProvider = addTidyChecks("google-explicit-constructor");
   EXPECT_THAT(
   *TU.build().getDiagnostics(),
   ElementsAre(
+  // Make sure the whole token is highlighted.
+  AllOf(Diag(Test.range("range"),
+ "invalid range expression of type 'struct Container *'; "
+ "did you mean to dereference it with '*'?"),
+WithFix(Fix(Test.range("insertstar"), "*", "insert '*'"))),
   // This range spans lines.
   AllOf(Diag(Test.range("typo"),
  "use of undeclared identifier 'goo'; did you mean 'foo'?"),
@@ -163,13 +183,7 @@
   AllOf(Diag(Test.range("macro"),
  "use of undeclared identifier 'fod'; did you mean 'foo'?"),
 WithFix(Fix(Test.range("macroarg"), "foo",
-"change 'fod' to 'foo'"))),
-  // We make sure here that the entire token is highlighted
-  AllOf(Diag(Test.range("constructor"),
- "single-argument constructors must be marked explicit to "
- "avoid unintentional implicit conversions"),
-WithFix(Fix(Test.range("explicit"), "explicit ",
-"insert 'explicit '");
+"change 'fod' to 'foo'");
 }
 
 TEST(DiagnosticsTest, FlagsMatter) {
@@ -212,10 +226,10 @@
   // Verify that we filter out the duplicated diagnostic message.
   EXPECT_THAT(
   *TU.build().getDiagnostics(),
-  UnorderedElementsAre(::testing::AllOf(
+  ifTidyChecks(UnorderedElementsAre(::testing::AllOf(
   Diag(Test.range(),
"floating point literal has suffix 'f', which is not uppercase"),
-  DiagSource(Diag::ClangTidy;
+  DiagSource(Diag::ClangTidy);
 
   Test = Annotations(R"cpp(
 template
@@ -232,10 +246,10 @@
   // duplicated messages, verify that we deduplicate them.
   EXPECT_THAT(
   *TU.build().getDiagnostics(),
-  UnorderedElementsAre(::testing::AllOf(
+  ifTidyChecks(UnorderedElementsAre(::testing::AllOf(
   Diag(Test.range(),
"floating point literal has suffix 'f', which is not uppercase"),
-  DiagSource(Diag::ClangTidy;
+  DiagSource(Diag::ClangTidy);
 }
 
 TEST(DiagnosticsTest, ClangTidy) {
@@ -249,6 +263,8 @@
   return $doubled[[sizeof]](sizeof(int));
 }
 
+class T{$explicit[[]]$constructor[[T]](int a);};
+
 // misc-no-recursion uses a custom traversal from the TUDecl
 void foo();
 void $bar[[bar]]() {
@@ -262,12 +278,14 @@
   TU.HeaderFilename = "assert.h"; // Supp

[PATCH] D103096: [analyzer] Implement cast for ranges of symbolic integers.

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov added inline comments.



Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:1203
+if (!Opts.ShouldSupportSymbolicIntegerCasts)
+  return VisitSymExpr(Sym);
+

vsavchenko wrote:
> Why do you use `VisitSymExpr` here?  You want to interrupt all `Visits or... 
> I'm not sure I fully understand.
Here we want to delegate the reasoning to another handler as we don't support 
non-integral cast yet.



Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:1215
+  QualType T = RootSym->getType();
+  if (!T->isIntegralOrEnumerationType())
+return VisitSymExpr(Sym);

vsavchenko wrote:
> Can we get a test for that?
I'll add some.


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

https://reviews.llvm.org/D103096

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

[PATCH] D105881: [flang][driver] Switch to `BoolFOption` for boolean options

[Andrzej Warzynski via Phabricator via cfe-commits]

awarzynski added inline comments.



Comment at: clang/include/clang/Driver/Options.td:246
 
+class EmptyKPM : KeyPathAndMacro<"", "", ""> {}
 class DiagnosticOpts

jansvoboda11 wrote:
> awarzynski wrote:
> > @jansvoboda11 , is this the right approach here? I'd like use `BoolFOption` 
> > in Flang, but we don't need `KeyPathAndMacro` just yet. We may do in the 
> > future.
> There was an `EmptyKPM` class present in the code before, but only as an 
> implementation detail of the marshalling system. People started using it with 
> `BoolFOption` to conveniently declare pairs Clang driver flags. I'm not a fan 
> since the whole `BooFOption` is designed around the keypath and its 
> marshalling. Putting a "null" keypath in `BoolFOption` and then talking about 
> its defaults and the effect of each flag seems highly unintuitive to me and 
> it's essentially dead code. I'd prefer a different approach in this patch 
> unless you have plans to switch to the marshalling infrastructure and remove 
> `EmptyKPM` in the short term.
> 
> Would something similar to `OptInFFlag` and `OptOutFFlag` work for you? You 
> could generalize it (remove `Flags<[CC1Option]>`), make use of it for Flang's 
> options directly and forward the current `Opt{In,Out}FFlag` to the 
> generalization while specifying the `CC1Option`. WDYT?
Thanks for the quick feedback! Initially when I saw `CC1Option` in 
`CC1Option`/`OptOutFFlag`, I immediately decided to explore alternatives. But I 
agree with everything that you said and am happy to generalize these 
multiclasses (instead of adding `EmptyKPM`). I don't see why it wouldn't work 
for what we currently need in Flang. I should be able to update this patch 
tomorrow.

Btw, what's the key benefit of the marshaling infra? Is that for being able to 
restore the original compiler invocation from instances of 
`CompilerInvocation`? We are still busy with other things, but definitely don't 
want Flang to miss out here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105881

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

[PATCH] D105890: Add paranthesis around logical expression to silence -Wlogical-op-parentheses warning.

[Bogdan Graur via Phabricator via cfe-commits]

bgraur created this revision.
Herald added a reviewer: aaron.ballman.
bgraur requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D105890

Files:
  clang/lib/Sema/SemaDeclAttr.cpp


Index: clang/lib/Sema/SemaDeclAttr.cpp
===
--- clang/lib/Sema/SemaDeclAttr.cpp
+++ clang/lib/Sema/SemaDeclAttr.cpp
@@ -7405,8 +7405,8 @@
   if (const auto *PDecl = dyn_cast(D)) {
 const Type *DeclTy = PDecl->getType().getCanonicalType().getTypePtr();
 if (AL.getAttrName()->getName().find("read_write") != StringRef::npos) {
-  if ((!S.getLangOpts().OpenCLCPlusPlus &&
-   (S.getLangOpts().OpenCLVersion < 200) ||
+  if (((!S.getLangOpts().OpenCLCPlusPlus &&
+(S.getLangOpts().OpenCLVersion < 200)) ||
(S.getLangOpts().OpenCLVersion == 300 &&
 !S.getOpenCLOptions().isSupported("__opencl_c_read_write_images",
   S.getLangOpts( ||


Index: clang/lib/Sema/SemaDeclAttr.cpp
===
--- clang/lib/Sema/SemaDeclAttr.cpp
+++ clang/lib/Sema/SemaDeclAttr.cpp
@@ -7405,8 +7405,8 @@
   if (const auto *PDecl = dyn_cast(D)) {
 const Type *DeclTy = PDecl->getType().getCanonicalType().getTypePtr();
 if (AL.getAttrName()->getName().find("read_write") != StringRef::npos) {
-  if ((!S.getLangOpts().OpenCLCPlusPlus &&
-   (S.getLangOpts().OpenCLVersion < 200) ||
+  if (((!S.getLangOpts().OpenCLCPlusPlus &&
+(S.getLangOpts().OpenCLVersion < 200)) ||
(S.getLangOpts().OpenCLVersion == 300 &&
 !S.getOpenCLOptions().isSupported("__opencl_c_read_write_images",
   S.getLangOpts( ||
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D105365: [Lexer] Fix bug in `makeFileCharRange` called on split tokens.

[Sam McCall via Phabricator via cfe-commits]

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

Sorry about this. I stared at this a few times and it *seems* right to me, and 
fixes the bug...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105365

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

[PATCH] D105457: [clang] Refactor AST printing tests to share more infrastructure

[John Paul Adrian Glaubitz via Phabricator via cfe-commits]

glaubitz added a comment.

In D105457#2873294 , @dyung wrote:

> This change I suspect is causing a build failure on a build bot.
>
> https://lab.llvm.org/buildbot/#/builders/110/builds/4827
>
>   FAILED: /usr/bin/c++  -DCLANG_ROUND_TRIP_CC1_ARGS=ON -DGTEST_HAS_RTTI=0 
> -D_DEBUG -D_GNU_SOURCE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS 
> -D__STDC_LIMIT_MACROS -Itools/clang/unittests/AST 
> -I/home/ssglocal/clang-cmake-x86_64-avx2-linux/clang-cmake-x86_64-avx2-linux/llvm/clang/unittests/AST
>  
> -I/home/ssglocal/clang-cmake-x86_64-avx2-linux/clang-cmake-x86_64-avx2-linux/llvm/clang/include
>  -Itools/clang/include -Iinclude 
> -I/home/ssglocal/clang-cmake-x86_64-avx2-linux/clang-cmake-x86_64-avx2-linux/llvm/llvm/include
>  
> -I/home/ssglocal/clang-cmake-x86_64-avx2-linux/clang-cmake-x86_64-avx2-linux/llvm/llvm/utils/unittest/googletest/include
>  
> -I/home/ssglocal/clang-cmake-x86_64-avx2-linux/clang-cmake-x86_64-avx2-linux/llvm/llvm/utils/unittest/googlemock/include
>  -march=broadwell -fPIC -fno-semantic-interposition 
> -fvisibility-inlines-hidden -Werror=date-time -Wall -Wextra 
> -Wno-unused-parameter -Wwrite-strings -Wcast-qual 
> -Wno-missing-field-initializers -pedantic -Wno-long-long 
> -Wno-maybe-uninitialized -Wdelete-non-virtual-dtor -Wno-comment 
> -fdiagnostics-color -ffunction-sections -fdata-sections -fno-common 
> -Woverloaded-virtual -fno-strict-aliasing -O3 -DNDEBUG
> -Wno-variadic-macros -fno-exceptions -fno-rtti -UNDEBUG -Wno-suggest-override 
> -std=c++14 -MD -MT 
> tools/clang/unittests/AST/CMakeFiles/ASTTests.dir/StmtPrinterTest.cpp.o -MF 
> tools/clang/unittests/AST/CMakeFiles/ASTTests.dir/StmtPrinterTest.cpp.o.d -o 
> tools/clang/unittests/AST/CMakeFiles/ASTTests.dir/StmtPrinterTest.cpp.o -c 
> /home/ssglocal/clang-cmake-x86_64-avx2-linux/clang-cmake-x86_64-avx2-linux/llvm/clang/unittests/AST/StmtPrinterTest.cpp
>   /tmp/cc8Ycpsn.s: Assembler messages:
>   /tmp/cc8Ycpsn.s:11414: Error: symbol 
> `_ZNSt17_Function_handlerIFbPKN5clang4StmtEENS0_UlS3_E2_EE9_M_invokeERKSt9_Any_dataOS3_'
>  is already defined
>   /tmp/cc8Ycpsn.s:11937: Error: symbol 
> `_ZNSt14_Function_base13_Base_managerIN5clangUlPKNS1_4StmtEE2_EE10_M_managerERSt9_Any_dataRKS7_St18_Manager_operation'
>  is already defined
>   ninja: build stopped: subcommand failed.
>
> Can you take a look?

Indeed, it also breaks the cross-build for M68k: 
http://lab.llvm.org:8014/#/builders/180/builds/341


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105457

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

[PATCH] D98710: [clang-tidy] New feature --skip-headers, part 1

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

In D98710#2873141 , @chh wrote:

> Sam, the revision summary is updated. Could you review it again?

Just to clarify - the code hasn't changed since last review pass right? I think 
the comments there still apply. Happy to review the patch description though!

> This new feature has impacts similar to --header-filter and --system-headers.

I don't think it does a similar thing to those flags, rather it mostly affects 
how those flags work. (The stuff about diagnostic counts is details that aren't 
really the "top-line" of this feature).
I'd say rather something like:

> The `--skip-headers` mode changes how the flags `--header-filter` and 
> `--system-headers` limit the scope of checks.`
> With `--skip-headers=0` (old behaviour; default), checkers inspect all code, 
> but warnings in files out of scope are not reported.
> With `--skip-headers=1`, checkers do not inspect code from files that are out 
> of scope. This can be a significant performance improvement.



> Add a --show-all-warnings flag. It is hidden, not shown by --help. When it is 
> used, ClangTidyDiagnosticConsumer::checkFilters will not suppress any 
> diagnostic message. This is useful to find tidy checks that have not yet 
> handled the --skip-headers flag.

This doesn't look right - an important part of the design is that tidy checks 
shouldn't need to be modified. (Though it's possible to imagine checks that 
wouldn't work properly in this mode)
Maybe rather "useful in conjunction with --skip-headers to find checks that may 
be processing code that should rather be skipped"?

> If there is such a case in the future, we might need some other method to 
> skip checks without cutting out Decls in AST

Realistically, I think we should probably just say such cases are unsupported. 
We believe no such checks exist in-tree, and clang-tidy upstream can't really 
be constrained by what people are doing out-of-tree.


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

https://reviews.llvm.org/D98710

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

[PATCH] D103096: [analyzer] Implement cast for ranges of symbolic integers.

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov added inline comments.



Comment at: 
clang/include/clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h:293-296
+SymbolRef Sym = Operand;
+while (isa(Sym))
+  Sym = cast(Sym)->Operand;
+return Sym;

vsavchenko wrote:
> 
Do you think the recursive call is better than the loop? But, I guess, I see 
your point. You option could be safer if we had another implementation of the 
virtual method. Or you think such alike cast symbol is possible in the future? 
Well, for now `ignoreCasts` doesn't make sense to any other `Expr` successors.


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

https://reviews.llvm.org/D103096

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

[PATCH] D104915: [OpenCL] Add support of __opencl_c_read_write_images feature macro

[Nico Weber via Phabricator via cfe-commits]

thakis added inline comments.



Comment at: clang/lib/Sema/SemaDeclAttr.cpp:7412
+!S.getOpenCLOptions().isSupported("__opencl_c_read_write_images",
+  S.getLangOpts( ||
   DeclTy->isPipeType()) {

Are the parens right here? You probably want

  `!S.getLangOpts().OpenCLCPlusPlus && (Ver < 200 || (Ver == 300 && 
isSupported))`

for the first term, but you have

  `!S.getLangOpts().OpenCLCPlusPlus && (Ver < 200) || (Ver == 300 && 
isSupported)`

Which means the OpenCLCPlusPlus only matters for `(Ver < 200)` and the `(Ver == 
300 && ...)` term makes things true independent of the OpenCLCPlusPlus check.

If what you currently have is what you want: Remove the parens around 
`(S.getLangOpts().OpenCLVersion < 200)` and do instead parenthesize like so: 
`(!S.getLangOpts().OpenCLCPlusPlus && Ver < 200)`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104915

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

[PATCH] D105221: [openmp][nfc] Simplify macros guarding math complex headers

[Jon Chesterfield via Phabricator via cfe-commits]

JonChesterfield updated this revision to Diff 358248.
JonChesterfield added a comment.

- reduce patch to only dropping cuda define


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105221

Files:
  clang/lib/Headers/__clang_cuda_complex_builtins.h
  clang/lib/Headers/openmp_wrappers/complex
  clang/lib/Headers/openmp_wrappers/complex.h


Index: clang/lib/Headers/openmp_wrappers/complex.h
===
--- clang/lib/Headers/openmp_wrappers/complex.h
+++ clang/lib/Headers/openmp_wrappers/complex.h
@@ -17,7 +17,6 @@
 // We require math functions in the complex builtins below.
 #include 
 
-#define __CUDA__
 #define __OPENMP_NVPTX__
 #include <__clang_cuda_complex_builtins.h>
 #undef __OPENMP_NVPTX__
Index: clang/lib/Headers/openmp_wrappers/complex
===
--- clang/lib/Headers/openmp_wrappers/complex
+++ clang/lib/Headers/openmp_wrappers/complex
@@ -17,7 +17,6 @@
 // We require std::math functions in the complex builtins below.
 #include 
 
-#define __CUDA__
 #define __OPENMP_NVPTX__
 #include <__clang_cuda_complex_builtins.h>
 #undef __OPENMP_NVPTX__
@@ -26,9 +25,6 @@
 // Grab the host header too.
 #include_next 
 
-
-#ifdef __cplusplus
-
 // If we are compiling against libc++, the macro _LIBCPP_STD_VER should be set
 // after including  above. Since the complex header we use is a
 // simplified version of the libc++, we don't need it in this case. If we
@@ -48,5 +44,3 @@
 #pragma omp end declare variant
 
 #endif
-
-#endif
Index: clang/lib/Headers/__clang_cuda_complex_builtins.h
===
--- clang/lib/Headers/__clang_cuda_complex_builtins.h
+++ clang/lib/Headers/__clang_cuda_complex_builtins.h
@@ -16,7 +16,7 @@
 // to work with CUDA and OpenMP target offloading [in C and C++ mode].)
 
 #pragma push_macro("__DEVICE__")
-#ifdef __OPENMP_NVPTX__
+#ifdef _OPENMP
 #pragma omp declare target
 #define __DEVICE__ __attribute__((noinline, nothrow, cold, weak))
 #else
@@ -26,7 +26,7 @@
 // To make the algorithms available for C and C++ in CUDA and OpenMP we select
 // different but equivalent function versions. TODO: For OpenMP we currently
 // select the native builtins as the overload support for templates is lacking.
-#if !defined(__OPENMP_NVPTX__)
+#if !defined(_OPENMP)
 #define _ISNANd std::isnan
 #define _ISNANf std::isnan
 #define _ISINFd std::isinf
@@ -276,7 +276,7 @@
 #undef _fmaxd
 #undef _fmaxf
 
-#ifdef __OPENMP_NVPTX__
+#ifdef _OPENMP
 #pragma omp end declare target
 #endif
 


Index: clang/lib/Headers/openmp_wrappers/complex.h
===
--- clang/lib/Headers/openmp_wrappers/complex.h
+++ clang/lib/Headers/openmp_wrappers/complex.h
@@ -17,7 +17,6 @@
 // We require math functions in the complex builtins below.
 #include 
 
-#define __CUDA__
 #define __OPENMP_NVPTX__
 #include <__clang_cuda_complex_builtins.h>
 #undef __OPENMP_NVPTX__
Index: clang/lib/Headers/openmp_wrappers/complex
===
--- clang/lib/Headers/openmp_wrappers/complex
+++ clang/lib/Headers/openmp_wrappers/complex
@@ -17,7 +17,6 @@
 // We require std::math functions in the complex builtins below.
 #include 
 
-#define __CUDA__
 #define __OPENMP_NVPTX__
 #include <__clang_cuda_complex_builtins.h>
 #undef __OPENMP_NVPTX__
@@ -26,9 +25,6 @@
 // Grab the host header too.
 #include_next 
 
-
-#ifdef __cplusplus
-
 // If we are compiling against libc++, the macro _LIBCPP_STD_VER should be set
 // after including  above. Since the complex header we use is a
 // simplified version of the libc++, we don't need it in this case. If we
@@ -48,5 +44,3 @@
 #pragma omp end declare variant
 
 #endif
-
-#endif
Index: clang/lib/Headers/__clang_cuda_complex_builtins.h
===
--- clang/lib/Headers/__clang_cuda_complex_builtins.h
+++ clang/lib/Headers/__clang_cuda_complex_builtins.h
@@ -16,7 +16,7 @@
 // to work with CUDA and OpenMP target offloading [in C and C++ mode].)
 
 #pragma push_macro("__DEVICE__")
-#ifdef __OPENMP_NVPTX__
+#ifdef _OPENMP
 #pragma omp declare target
 #define __DEVICE__ __attribute__((noinline, nothrow, cold, weak))
 #else
@@ -26,7 +26,7 @@
 // To make the algorithms available for C and C++ in CUDA and OpenMP we select
 // different but equivalent function versions. TODO: For OpenMP we currently
 // select the native builtins as the overload support for templates is lacking.
-#if !defined(__OPENMP_NVPTX__)
+#if !defined(_OPENMP)
 #define _ISNANd std::isnan
 #define _ISNANf std::isnan
 #define _ISINFd std::isinf
@@ -276,7 +276,7 @@
 #undef _fmaxd
 #undef _fmaxf
 
-#ifdef __OPENMP_NVPTX__
+#ifdef _OPENMP
 #pragma omp end declare target
 #endif
 
___
cfe-commits mail

[PATCH] D105890: [NFC] Add paranthesis around logical expression to silence -Wlogical-op-parentheses warning.

[Alexander Kornienko via Phabricator via cfe-commits]

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

LG. I can commit the patch for you.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105890

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

[clang] 45ffe63 - [clang/objc] Optimize getters for non-atomic, copied properties

[Nico Weber via cfe-commits]

Author: Dave MacLachlan
Date: 2021-07-13T09:22:13-04:00
New Revision: 45ffe6341d9642487785b0d0028166e6fbdbe5d7

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

LOG: [clang/objc] Optimize getters for non-atomic, copied properties

Properties that were declared `@property(copy, nonatomic) id foo` make an
unnecessary call to objc_get_property().  This call can be replaced with a
direct access to the backing variable identical to how a `@property(nonatomic)
id foo` would do it.

This reduces codegen by 4 bytes (x86_64/arm64) and removes a cross linkage unit
function call per property declared as copy/nonatomic.

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

Added: 


Modified: 
clang/lib/CodeGen/CGObjC.cpp
clang/test/CodeGenObjC/arc-blocks.m

Removed: 




diff  --git a/clang/lib/CodeGen/CGObjC.cpp b/clang/lib/CodeGen/CGObjC.cpp
index b865780ffe93c..2f0acd440e50d 100644
--- a/clang/lib/CodeGen/CGObjC.cpp
+++ b/clang/lib/CodeGen/CGObjC.cpp
@@ -925,10 +925,11 @@ PropertyImplStrategy::PropertyImplStrategy(CodeGenModule 
&CGM,
   IvarSize = TInfo.Width;
   IvarAlignment = TInfo.Align;
 
-  // If we have a copy property, we always have to use getProperty/setProperty.
-  // TODO: we could actually use setProperty and an expression for non-atomics.
+  // If we have a copy property, we always have to use setProperty.
+  // If the property is atomic we need to use getProperty, but in
+  // the nonatomic case we can just use expression.
   if (IsCopy) {
-Kind = GetSetProperty;
+Kind = IsAtomic ? GetSetProperty : SetPropertyAndExpressionGet;
 return;
   }
 

diff  --git a/clang/test/CodeGenObjC/arc-blocks.m 
b/clang/test/CodeGenObjC/arc-blocks.m
index 078408bc1d6c0..ab0ee3789cba8 100644
--- a/clang/test/CodeGenObjC/arc-blocks.m
+++ b/clang/test/CodeGenObjC/arc-blocks.m
@@ -477,7 +477,7 @@ @implementation Test12
 // CHECK:call void @objc_setProperty(i8* {{%.*}}, i8* {{%.*}}, i64 
{{%.*}}, i8* {{%.*}}, i1 zeroext true, i1 zeroext true)
 
 // CHECK:define internal void ()* @"\01-[Test12 nblock]"(
-// CHECK:call i8* @objc_getProperty(i8* {{%.*}}, i8* {{%.*}}, i64 {{%.*}}, 
i1 zeroext false)
+// CHECK:%add.ptr = getelementptr inbounds i8, i8* %1, i64 %ivar
 
 // CHECK:define internal void @"\01-[Test12 setNblock:]"(
 // CHECK:call void @objc_setProperty(i8* {{%.*}}, i8* {{%.*}}, i64 
{{%.*}}, i8* {{%.*}}, i1 zeroext false, i1 zeroext true)



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

[PATCH] D105311: Optimize getters for non-atomic, copied properties

[Nico Weber via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG45ffe6341d96: [clang/objc] Optimize getters for non-atomic, 
copied properties (authored by dmaclach, committed by thakis).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105311

Files:
  clang/lib/CodeGen/CGObjC.cpp
  clang/test/CodeGenObjC/arc-blocks.m


Index: clang/test/CodeGenObjC/arc-blocks.m
===
--- clang/test/CodeGenObjC/arc-blocks.m
+++ clang/test/CodeGenObjC/arc-blocks.m
@@ -477,7 +477,7 @@
 // CHECK:call void @objc_setProperty(i8* {{%.*}}, i8* {{%.*}}, i64 
{{%.*}}, i8* {{%.*}}, i1 zeroext true, i1 zeroext true)
 
 // CHECK:define internal void ()* @"\01-[Test12 nblock]"(
-// CHECK:call i8* @objc_getProperty(i8* {{%.*}}, i8* {{%.*}}, i64 {{%.*}}, 
i1 zeroext false)
+// CHECK:%add.ptr = getelementptr inbounds i8, i8* %1, i64 %ivar
 
 // CHECK:define internal void @"\01-[Test12 setNblock:]"(
 // CHECK:call void @objc_setProperty(i8* {{%.*}}, i8* {{%.*}}, i64 
{{%.*}}, i8* {{%.*}}, i1 zeroext false, i1 zeroext true)
Index: clang/lib/CodeGen/CGObjC.cpp
===
--- clang/lib/CodeGen/CGObjC.cpp
+++ clang/lib/CodeGen/CGObjC.cpp
@@ -925,10 +925,11 @@
   IvarSize = TInfo.Width;
   IvarAlignment = TInfo.Align;
 
-  // If we have a copy property, we always have to use getProperty/setProperty.
-  // TODO: we could actually use setProperty and an expression for non-atomics.
+  // If we have a copy property, we always have to use setProperty.
+  // If the property is atomic we need to use getProperty, but in
+  // the nonatomic case we can just use expression.
   if (IsCopy) {
-Kind = GetSetProperty;
+Kind = IsAtomic ? GetSetProperty : SetPropertyAndExpressionGet;
 return;
   }
 


Index: clang/test/CodeGenObjC/arc-blocks.m
===
--- clang/test/CodeGenObjC/arc-blocks.m
+++ clang/test/CodeGenObjC/arc-blocks.m
@@ -477,7 +477,7 @@
 // CHECK:call void @objc_setProperty(i8* {{%.*}}, i8* {{%.*}}, i64 {{%.*}}, i8* {{%.*}}, i1 zeroext true, i1 zeroext true)
 
 // CHECK:define internal void ()* @"\01-[Test12 nblock]"(
-// CHECK:call i8* @objc_getProperty(i8* {{%.*}}, i8* {{%.*}}, i64 {{%.*}}, i1 zeroext false)
+// CHECK:%add.ptr = getelementptr inbounds i8, i8* %1, i64 %ivar
 
 // CHECK:define internal void @"\01-[Test12 setNblock:]"(
 // CHECK:call void @objc_setProperty(i8* {{%.*}}, i8* {{%.*}}, i64 {{%.*}}, i8* {{%.*}}, i1 zeroext false, i1 zeroext true)
Index: clang/lib/CodeGen/CGObjC.cpp
===
--- clang/lib/CodeGen/CGObjC.cpp
+++ clang/lib/CodeGen/CGObjC.cpp
@@ -925,10 +925,11 @@
   IvarSize = TInfo.Width;
   IvarAlignment = TInfo.Align;
 
-  // If we have a copy property, we always have to use getProperty/setProperty.
-  // TODO: we could actually use setProperty and an expression for non-atomics.
+  // If we have a copy property, we always have to use setProperty.
+  // If the property is atomic we need to use getProperty, but in
+  // the nonatomic case we can just use expression.
   if (IsCopy) {
-Kind = GetSetProperty;
+Kind = IsAtomic ? GetSetProperty : SetPropertyAndExpressionGet;
 return;
   }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D105221: [openmp][nfc] Simplify macros guarding math complex headers

[Jon Chesterfield via Phabricator via cfe-commits]

JonChesterfield updated this revision to Diff 358250.
JonChesterfield added a comment.

- reduce patch to only dropping cuda define v2, now with missing save


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105221

Files:
  clang/lib/Headers/openmp_wrappers/complex
  clang/lib/Headers/openmp_wrappers/complex.h


Index: clang/lib/Headers/openmp_wrappers/complex.h
===
--- clang/lib/Headers/openmp_wrappers/complex.h
+++ clang/lib/Headers/openmp_wrappers/complex.h
@@ -17,7 +17,6 @@
 // We require math functions in the complex builtins below.
 #include 
 
-#define __CUDA__
 #define __OPENMP_NVPTX__
 #include <__clang_cuda_complex_builtins.h>
 #undef __OPENMP_NVPTX__
Index: clang/lib/Headers/openmp_wrappers/complex
===
--- clang/lib/Headers/openmp_wrappers/complex
+++ clang/lib/Headers/openmp_wrappers/complex
@@ -17,7 +17,6 @@
 // We require std::math functions in the complex builtins below.
 #include 
 
-#define __CUDA__
 #define __OPENMP_NVPTX__
 #include <__clang_cuda_complex_builtins.h>
 #undef __OPENMP_NVPTX__
@@ -26,9 +25,6 @@
 // Grab the host header too.
 #include_next 
 
-
-#ifdef __cplusplus
-
 // If we are compiling against libc++, the macro _LIBCPP_STD_VER should be set
 // after including  above. Since the complex header we use is a
 // simplified version of the libc++, we don't need it in this case. If we
@@ -48,5 +44,3 @@
 #pragma omp end declare variant
 
 #endif
-
-#endif


Index: clang/lib/Headers/openmp_wrappers/complex.h
===
--- clang/lib/Headers/openmp_wrappers/complex.h
+++ clang/lib/Headers/openmp_wrappers/complex.h
@@ -17,7 +17,6 @@
 // We require math functions in the complex builtins below.
 #include 
 
-#define __CUDA__
 #define __OPENMP_NVPTX__
 #include <__clang_cuda_complex_builtins.h>
 #undef __OPENMP_NVPTX__
Index: clang/lib/Headers/openmp_wrappers/complex
===
--- clang/lib/Headers/openmp_wrappers/complex
+++ clang/lib/Headers/openmp_wrappers/complex
@@ -17,7 +17,6 @@
 // We require std::math functions in the complex builtins below.
 #include 
 
-#define __CUDA__
 #define __OPENMP_NVPTX__
 #include <__clang_cuda_complex_builtins.h>
 #undef __OPENMP_NVPTX__
@@ -26,9 +25,6 @@
 // Grab the host header too.
 #include_next 
 
-
-#ifdef __cplusplus
-
 // If we are compiling against libc++, the macro _LIBCPP_STD_VER should be set
 // after including  above. Since the complex header we use is a
 // simplified version of the libc++, we don't need it in this case. If we
@@ -48,5 +44,3 @@
 #pragma omp end declare variant
 
 #endif
-
-#endif
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D105221: [openmp][nfc] Simplify macros guarding math complex headers

[Jon Chesterfield via Phabricator via cfe-commits]

JonChesterfield added a comment.

Cut down to only dropping the cuda define, which is sufficient to resolve 
D104904 . Haven't build/tested this diff yet.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105221

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

[PATCH] D103096: [analyzer] Implement cast for ranges of symbolic integers.

[Valeriy Savchenko via Phabricator via cfe-commits]

vsavchenko added a comment.

I'll allocate some time to get into your summary, but for now here are my 
concerns about `SymbolRangeInferrer` and `VisitSymbolCast`.




Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:1203
+if (!Opts.ShouldSupportSymbolicIntegerCasts)
+  return VisitSymExpr(Sym);
+

ASDenysPetrov wrote:
> vsavchenko wrote:
> > Why do you use `VisitSymExpr` here?  You want to interrupt all `Visits 
> > or... I'm not sure I fully understand.
> Here we want to delegate the reasoning to another handler as we don't support 
> non-integral cast yet.
You are not delegating it here.  `Visit` includes a runtime dispatch that calls 
a correct `VisitTYPE` method.  Here you call `VisitSymExpr` directly, which is 
one of the `VisitTYPE` methods.  No dispatch will happen, and since we use 
`VisitSymExpr` as the last resort (it's the most base class, if we got there, 
we don't actually support the expression), you interrupt the `Visit` and go 
directly to "the last resort".

See the problem now?



Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:1241
+  // Find the first constraint and exit the loop.
+  RSPtr = getConstraint(State, S);
+}

ASDenysPetrov wrote:
> vsavchenko wrote:
> > Why do you get associated constraint directly without consulting with what 
> > `SymbolRangeInferrer` can tell you about it?
> What do you mean? I didn't get. Could you give an example?
`getConstraint` returns whatever constraint we have stored directly in the 
constraint map.  That's the main source of information for ranges, but not the 
only one.

Here is the  of things that you skip, when you do `getConstraint` here:
  * we can understand that something is equality/disequality check and find the 
corresponding info in Equivalence Classes data structure
  * we can see that the expression has the form `A - B` and we can find 
constraint for `B - A`
  * we can see that the expression is comparison `A op B` and check what other 
comparison info we have on `A` and `B` (your own change)
  * we can see that the expression is of form `A op B` and check if we know 
something about `A` and `B`, and produce a reasonable constraint out of this 
information

In order to use the right information, you should use `infer` that will 
actually do all other things as well.  That's how `SymbolRangeInferrer` is 
designed, to be **recursive**.

Speaking of **recursiveness**.  All these loops and manually checking for types 
of the cast's operand is against this pattern.  Recursive visitors should call 
`Visit` for children nodes (like `RecursiveASTVisitor`).  In other words, if 
`f(x)` is a visit function, it should be defined like this:
```
f(x) = g(f(x->operand_1), f(x->operand_2), ... , f(x->operand_N))
```
or if we talk about your case specifically:
```
f(x: SymbolCast) = h(f(x->Operand))
```
and the `h` function should transform the range set returned by `f(x->Operand)` 
into a range set appropriate for `x`.

NOTE: `h` can also intersect different ranges


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

https://reviews.llvm.org/D103096

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

[PATCH] D105892: [NFC] Silence build warning by placing parentheses around condition

[Anton Zabaznov via Phabricator via cfe-commits]

azabaznov created this revision.
azabaznov added reviewers: Anastasia, thakis.
Herald added a reviewer: aaron.ballman.
azabaznov requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D105892

Files:
  clang/lib/Sema/SemaDeclAttr.cpp


Index: clang/lib/Sema/SemaDeclAttr.cpp
===
--- clang/lib/Sema/SemaDeclAttr.cpp
+++ clang/lib/Sema/SemaDeclAttr.cpp
@@ -7406,10 +7406,10 @@
 const Type *DeclTy = PDecl->getType().getCanonicalType().getTypePtr();
 if (AL.getAttrName()->getName().find("read_write") != StringRef::npos) {
   if ((!S.getLangOpts().OpenCLCPlusPlus &&
-   (S.getLangOpts().OpenCLVersion < 200) ||
-   (S.getLangOpts().OpenCLVersion == 300 &&
-!S.getOpenCLOptions().isSupported("__opencl_c_read_write_images",
-  S.getLangOpts( ||
+   ((S.getLangOpts().OpenCLVersion < 200) ||
+(S.getLangOpts().OpenCLVersion == 300 &&
+ !S.getOpenCLOptions().isSupported("__opencl_c_read_write_images",
+   S.getLangOpts() ||
   DeclTy->isPipeType()) {
 S.Diag(AL.getLoc(), diag::err_opencl_invalid_read_write)
 << AL << PDecl->getType() << DeclTy->isImageType();


Index: clang/lib/Sema/SemaDeclAttr.cpp
===
--- clang/lib/Sema/SemaDeclAttr.cpp
+++ clang/lib/Sema/SemaDeclAttr.cpp
@@ -7406,10 +7406,10 @@
 const Type *DeclTy = PDecl->getType().getCanonicalType().getTypePtr();
 if (AL.getAttrName()->getName().find("read_write") != StringRef::npos) {
   if ((!S.getLangOpts().OpenCLCPlusPlus &&
-   (S.getLangOpts().OpenCLVersion < 200) ||
-   (S.getLangOpts().OpenCLVersion == 300 &&
-!S.getOpenCLOptions().isSupported("__opencl_c_read_write_images",
-  S.getLangOpts( ||
+   ((S.getLangOpts().OpenCLVersion < 200) ||
+(S.getLangOpts().OpenCLVersion == 300 &&
+ !S.getOpenCLOptions().isSupported("__opencl_c_read_write_images",
+   S.getLangOpts() ||
   DeclTy->isPipeType()) {
 S.Diag(AL.getLoc(), diag::err_opencl_invalid_read_write)
 << AL << PDecl->getType() << DeclTy->isImageType();
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D105890: [NFC] Add paranthesis around logical expression to silence -Wlogical-op-parentheses warning.

[Alexander Kornienko 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 rGe9533b849207: [NFC] Add paranthesis around logical 
expression to silence -Wlogical-op… (authored by bgraur, committed by alexfh).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105890

Files:
  clang/lib/Sema/SemaDeclAttr.cpp


Index: clang/lib/Sema/SemaDeclAttr.cpp
===
--- clang/lib/Sema/SemaDeclAttr.cpp
+++ clang/lib/Sema/SemaDeclAttr.cpp
@@ -7405,8 +7405,8 @@
   if (const auto *PDecl = dyn_cast(D)) {
 const Type *DeclTy = PDecl->getType().getCanonicalType().getTypePtr();
 if (AL.getAttrName()->getName().find("read_write") != StringRef::npos) {
-  if ((!S.getLangOpts().OpenCLCPlusPlus &&
-   (S.getLangOpts().OpenCLVersion < 200) ||
+  if (((!S.getLangOpts().OpenCLCPlusPlus &&
+(S.getLangOpts().OpenCLVersion < 200)) ||
(S.getLangOpts().OpenCLVersion == 300 &&
 !S.getOpenCLOptions().isSupported("__opencl_c_read_write_images",
   S.getLangOpts( ||


Index: clang/lib/Sema/SemaDeclAttr.cpp
===
--- clang/lib/Sema/SemaDeclAttr.cpp
+++ clang/lib/Sema/SemaDeclAttr.cpp
@@ -7405,8 +7405,8 @@
   if (const auto *PDecl = dyn_cast(D)) {
 const Type *DeclTy = PDecl->getType().getCanonicalType().getTypePtr();
 if (AL.getAttrName()->getName().find("read_write") != StringRef::npos) {
-  if ((!S.getLangOpts().OpenCLCPlusPlus &&
-   (S.getLangOpts().OpenCLVersion < 200) ||
+  if (((!S.getLangOpts().OpenCLCPlusPlus &&
+(S.getLangOpts().OpenCLVersion < 200)) ||
(S.getLangOpts().OpenCLVersion == 300 &&
 !S.getOpenCLOptions().isSupported("__opencl_c_read_write_images",
   S.getLangOpts( ||
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] e9533b8 - [NFC] Add paranthesis around logical expression to silence -Wlogical-op-parentheses warning.

[Alexander Kornienko via cfe-commits]

Author: Bogdan Graur
Date: 2021-07-13T15:54:31+02:00
New Revision: e9533b84920798cf9b35d26586a61bad0a1f9825

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

LOG: [NFC] Add paranthesis around logical expression to silence 
-Wlogical-op-parentheses warning.

Reviewed By: alexfh

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

Added: 


Modified: 
clang/lib/Sema/SemaDeclAttr.cpp

Removed: 




diff  --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp
index 87e5531a7705..a5d0597e39c4 100644
--- a/clang/lib/Sema/SemaDeclAttr.cpp
+++ b/clang/lib/Sema/SemaDeclAttr.cpp
@@ -7405,8 +7405,8 @@ static void handleOpenCLAccessAttr(Sema &S, Decl *D, 
const ParsedAttr &AL) {
   if (const auto *PDecl = dyn_cast(D)) {
 const Type *DeclTy = PDecl->getType().getCanonicalType().getTypePtr();
 if (AL.getAttrName()->getName().find("read_write") != StringRef::npos) {
-  if ((!S.getLangOpts().OpenCLCPlusPlus &&
-   (S.getLangOpts().OpenCLVersion < 200) ||
+  if (((!S.getLangOpts().OpenCLCPlusPlus &&
+(S.getLangOpts().OpenCLVersion < 200)) ||
(S.getLangOpts().OpenCLVersion == 300 &&
 !S.getOpenCLOptions().isSupported("__opencl_c_read_write_images",
   S.getLangOpts( ||



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

[PATCH] D104915: [OpenCL] Add support of __opencl_c_read_write_images feature macro

[Anton Zabaznov via Phabricator via cfe-commits]

azabaznov added inline comments.



Comment at: clang/lib/Sema/SemaDeclAttr.cpp:7412
+!S.getOpenCLOptions().isSupported("__opencl_c_read_write_images",
+  S.getLangOpts( ||
   DeclTy->isPipeType()) {

thakis wrote:
> Are the parens right here? You probably want
> 
>   `!S.getLangOpts().OpenCLCPlusPlus && (Ver < 200 || (Ver == 300 && 
> isSupported))`
> 
> for the first term, but you have
> 
>   `!S.getLangOpts().OpenCLCPlusPlus && (Ver < 200) || (Ver == 300 && 
> isSupported)`
> 
> Which means the OpenCLCPlusPlus only matters for `(Ver < 200)` and the `(Ver 
> == 300 && ...)` term makes things true independent of the OpenCLCPlusPlus 
> check.
> 
> If what you currently have is what you want: Remove the parens around 
> `(S.getLangOpts().OpenCLVersion < 200)` and do instead parenthesize like so: 
> `(!S.getLangOpts().OpenCLCPlusPlus && Ver < 200)`.
Thanks. I've submitted the patch: https://reviews.llvm.org/D105892. Please 
approve.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104915

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

[PATCH] D103096: [analyzer] Implement cast for ranges of symbolic integers.

[Valeriy Savchenko via Phabricator via cfe-commits]

vsavchenko added inline comments.



Comment at: 
clang/include/clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h:293-296
+SymbolRef Sym = Operand;
+while (isa(Sym))
+  Sym = cast(Sym)->Operand;
+return Sym;

ASDenysPetrov wrote:
> vsavchenko wrote:
> > 
> Do you think the recursive call is better than the loop? But, I guess, I see 
> your point. You option could be safer if we had another implementation of the 
> virtual method. Or you think such alike cast symbol is possible in the 
> future? Well, for now `ignoreCasts` doesn't make sense to any other `Expr` 
> successors.
Oh, wait, why is it even virtual?  I don't think that it should be virtual.
Are similar functions in `Expr` virtual?
And I think that this implementation should live in `SymExpr` directly.  Then 
it would look like:
```
if (const SymbolCast *ThisAsCast = dyn_cast(this)) {
  return ThisAsCast->ignoreCasts();
}
return this;
```


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

https://reviews.llvm.org/D103096

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

[PATCH] D105881: [flang][driver] Switch to `BoolFOption` for boolean options

[Pete Steinfeld via Phabricator via cfe-commits]

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

This built OK, but when I run `check-flang`, I see errors for 
`.../Driver/driver-help.f90` and `.../Driver/driver-help-hidden.f90`.  I'm 
building on a Linux system using GNU g++ version 9.3.  Here's an excerpt from 
the log file:

  FAIL: Flang :: Driver/driver-help.f90 (4 of 712)
   TEST 'Flang :: Driver/driver-help.f90' FAILED 

  Script:
  --
  : 'RUN: at line 6';   
/local/home/psteinfeld/main/andy/flang/build/bin/flang-new -help 2>&1 | 
/local/home/psteinfeld/main/clean/install/bin/FileCheck 
/local/home/psteinfeld/main/andy/flang/test/Driver/driver-help.f90 
--check-prefix=HELP
  : 'RUN: at line 7';   not 
/local/home/psteinfeld/main/andy/flang/build/bin/flang-new -helps 2>&1 | 
/local/home/psteinfeld/main/clean/install/bin/FileCheck 
/local/home/psteinfeld/main/andy/flang/test/Driver/driver-help.f90 
--check-prefix=ERROR
  : 'RUN: at line 12';   
/local/home/psteinfeld/main/andy/flang/build/bin/flang-new -fc1 -help 2>&1 | 
/local/home/psteinfeld/main/clean/install/bin/FileCheck 
/local/home/psteinfeld/main/andy/flang/test/Driver/driver-help.f90 
--check-prefix=HELP-FC1
  : 'RUN: at line 13';   not 
/local/home/psteinfeld/main/andy/flang/build/bin/flang-new -fc1 -helps 2>&1 | 
/local/home/psteinfeld/main/clean/install/bin/FileCheck 
/local/home/psteinfeld/main/andy/flang/test/Driver/driver-help.f90 
--check-prefix=ERROR
  --
  Exit Code: 1
  
  Command Output (stderr):
  --
  /local/home/psteinfeld/main/andy/flang/test/Driver/driver-help.f90:43:14: 
error: HELP-NEXT: expected string not found in input
  ! HELP-NEXT: -fno-backslash
   ^
  :27:54: note: scanning from here
   -flogical-abbreviations Enable logical abbreviations
   ^
  :28:2: note: possible intended match here
   -fno-color-diagnostics Disable colors in diagnostics
   ^
  
  Input file: 
  Check file: /local/home/psteinfeld/main/andy/flang/test/Driver/driver-help.f90
  
  -dump-input=help explains the following input dump.
  
  Input was:
  <<
 .
 .
 .
22:  -fimplicit-none No implicit typing allowed unless overridden 
by IMPLICIT statements 
23:  -finput-charset= Specify the default character set for 
source files 
24:  -fintrinsic-modules-path  
25:  Specify where to find the compiled intrinsic modules 
26:  -flarge-sizes Use INTEGER(KIND=8) for the result type in 
size-related intrinsics 
27:  -flogical-abbreviations Enable logical abbreviations 
  next:43'0  X error: 
no match found
28:  -fno-color-diagnostics Disable colors in diagnostics 
  next:43'0 ~~
  next:43'1  ? possible 
intended match
29:  -fopenacc Enable OpenACC 
  next:43'0 ~~
30:  -fopenmp Parse OpenMP pragmas and generate parallel code. 
  next:43'0 ~~~
31:  -fxor-operator Enable .XOR. as a synonym of .NEQV. 
  next:43'0 
32:  -help Display available options 
  next:43'0 ~
33:  -I  Add directory to the end of the list of include 
search paths 
  next:43'0 
~~~
 .
 .
 .
  >>
  
  --
  
  
  FAIL: Flang :: Driver/driver-help-hidden.f90 (5 of 712)
   TEST 'Flang :: Driver/driver-help-hidden.f90' FAILED 

  Script:
  --
  : 'RUN: at line 6';   
/local/home/psteinfeld/main/andy/flang/build/bin/flang-new --help-hidden 2>&1 | 
/local/home/psteinfeld/main/clean/install/bin/FileCheck 
/local/home/psteinfeld/main/andy/flang/test/Driver/driver-help-hidden.f90
  : 'RUN: at line 7';   not 
/local/home/psteinfeld/main/andy/flang/build/bin/flang-new  -help-hidden 2>&1 | 
/local/home/psteinfeld/main/clean/install/bin/FileCheck 
/local/home/psteinfeld/main/andy/flang/test/Driver/driver-help-hidden.f90 
--check-prefix=ERROR-FLANG
  : 'RUN: at line 12';   not 
/local/home/psteinfeld/main/andy/flang/build/bin/flang-new -fc1 --help-hidden 
2>&1 | /local/home/psteinfeld/main/clean/install/bin/FileCheck 
/local/home/psteinfeld/main/andy/flang/test/Driver/driver-help-hidden.f90 
--check-prefix=ERROR-FLANG-FC1
  : 'RUN: at line 13';   not 
/local/home/psteinfeld/main/andy/flang/build/bin/flang-new -fc1  -help-hidden 
2>&1 | /local/home/psteinfeld/main/clean/install/bin/FileCheck 
/local/home/psteinfeld/main/andy/flang/test/Driver/driver-help-hidden.f90 
--check-prefix=ERROR

[PATCH] D105892: [NFC] Silence build warning by placing parentheses around condition

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

This is not NFC, is it? Before, this got evaluated as 
`(!S.getLangOpts().OpenCLCPlusPlus && S.getLangOpts().OpenCLVersion < 200) || 
(S.getLangOpts().OpenCLVersion == 300 && 
!S.getOpenCLOptions().isSupported("__opencl_c_read_write_images", 
S.getLangOpts()))`. Now, it's evaluated as `!S.getLangOpts().OpenCLCPlusPlus && 
(...rest...)` -- that is, `!S.getLangOpts().OpenCLCPlusPlus` used to be anded 
with just one term and now it's anded with the whole thing.

Maybe it's worth to introduce some bool variables to make this less confusing 
instead of this one very long term?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105892

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

[PATCH] D105679: [clangd] Add CMake option to (not) link in clang-tidy checks

[Kadir Cetinkaya via Phabricator via cfe-commits]

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

thanks, lgtm!




Comment at: clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp:259
 
+class T{$explicit[[]]$constructor[[T]](int a);};
+

sammccall wrote:
> kadircet wrote:
> > ah, should've kept reading before leaving the comment above. feel free to 
> > ignore that (or just drop this test and rely only on clang diagnostics for 
> > checking that behaviour?)
> This really belongs in the diagnostic ranges test, so switched to a different 
> diag as you suggested.
nit: is this testing anything useful now? maybe just drop it?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105679

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

[PATCH] D105892: [NFC] Silence build warning by placing parentheses around condition

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a reviewer: alexfh.
aaron.ballman added a comment.

Note: https://reviews.llvm.org/D105890 that was just landed to touch the same 
area.

FWIW, I agree that this should probably be using some local variables to make 
the conditions a bit more clear.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105892

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

[PATCH] D103986: [PowerPC] Floating Point Builtins for XL Compat.

[Nemanja Ivanovic via Phabricator via cfe-commits]

nemanjai added inline comments.



Comment at: clang/test/CodeGen/builtins-ppc-xlcompat-fp.c:5
+// RUN: %clang_cc1 -triple powerpc64le-unknown-unknown \
+// RUN:   -emit-llvm %s -o - -target-cpu pwr8 | FileCheck %s
+// RUN: %clang_cc1 -triple powerpc64-unknown-aix \

AFAICT, nothing changes with Power8 so you might as well just have run lines 
for Power7 (AIX) and Power8 (LE).



Comment at: llvm/include/llvm/IR/IntrinsicsPowerPC.td:1529
+ Intrinsic<[llvm_double_ty], [llvm_double_ty, 
+ llvm_double_ty, llvm_double_ty], [IntrNoMem]>;
+  def int_ppc_fsels : GCCBuiltin<"__builtin_ppc_fsels">,

amyk wrote:
> Minor indentation nit.
I don't think this indentation is desired. This makes it look like the first 
type on line 1529 belongs in the first list rather than the second list it 
belongs to. I suggest the following:
```
def int_ppc_fsel :
  GCCBuiltin<"__builtin_ppc_fsel">,
  Intrinsic<[llvm_double_ty], [llvm_double_ty,
   llvm_double_ty, llvm_double_ty], [IntrNoMem]>;
```



Comment at: llvm/lib/Target/PowerPC/PPCISelDAGToDAG.cpp:4990
   case ISD::INTRINSIC_WO_CHAIN: {
+
+if (N->getConstantOperandVal(0) == Intrinsic::ppc_fsels) {

NeHuang wrote:
> you can delete blank line and better add comments for the operation below.
I agree that this warrants a comment. However I don't think this is really what 
Victor had in mind. In general if your comment boils down to "this is what we 
are doing" and simply describes the code, it is not very useful. What your 
comment should address is "why are we doing this here and this way".

To me as a reader, it is not at all clear why we manually select this to 
`PPC::FSELS` here. All the other intrinsics are matched with patterns in the 
`.td` file but this one is matched specifically here. Why?



Comment at: llvm/test/CodeGen/builtins-ppc-xlcompat-fp.ll:2
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc -verify-machineinstrs -mtriple=powerpc64-unknown-unknown \
+; RUN:   -mcpu=pwr7 < %s | FileCheck %s --check-prefix=CHECK-PWR7

Again, this is excessive. There should be run lines for Power8, Power8-32-bit, 
Power8-no-vsx, Power7. We don't really need a cross-product of triples and CPUs.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103986

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

[PATCH] D102875: [PowerPC] Add PowerPC compare and multiply related builtins and instrinsics for XL compatibility

[Nemanja Ivanovic via Phabricator via cfe-commits]

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

Requesting changes to keep the queue accurate.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102875

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

[PATCH] D105890: [NFC] Add paranthesis around logical expression to silence -Wlogical-op-parentheses warning.

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

I think this is incorrect. See D105892  and 
https://reviews.llvm.org/D104915#2873851 . Don't blindly land changes to 
suppress warnings – the warnings exist to tell us something :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105890

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

[PATCH] D103986: [PowerPC] Floating Point Builtins for XL Compat.

[Nemanja Ivanovic via Phabricator via cfe-commits]

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

Requesting changes to remove it from the queue until comments are addressed or 
answered.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103986

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

[PATCH] D105660: [PowerPC][AIX] Add warning when alignment is incompatible with XL

[Zarko Todorovski via Phabricator via cfe-commits]

ZarkoCA updated this revision to Diff 358261.
ZarkoCA added a comment.

- Add a warning group for this warning


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105660

Files:
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/Sema/aix-attr-align.c


Index: clang/test/Sema/aix-attr-align.c
===
--- /dev/null
+++ clang/test/Sema/aix-attr-align.c
@@ -0,0 +1,17 @@
+// RUN: %clang_cc1 -triple powerpc64-ibm-aix-xcoff -verify -fsyntax-only %s
+// RUN: %clang_cc1 -triple powerpc-ibm-aix-xcoff -verify -fsyntax-only %s
+
+struct S {
+  int a[8] __attribute__((aligned(8)));  // no-warning
+};
+
+struct T {
+  int a[4] __attribute__((aligned(16))); // expected-warning {{requesting an 
alignment of arguments 16 bytes or greater is not binary compatible with AIX XL 
16.1 and older}}
+};
+
+struct U {
+  int a[2] __attribute__((aligned(32))); // expected-warning {{requesting an 
alignment of arguments 16 bytes or greater is not binary compatible with AIX XL 
16.1 and older}}
+};
+  int a[8] __attribute__((aligned(8)));  // no-warning
+  int b[4] __attribute__((aligned(16))); // no-warning 
+  int c[2] __attribute__((aligned(32))); // no-warning
Index: clang/lib/Sema/SemaDeclAttr.cpp
===
--- clang/lib/Sema/SemaDeclAttr.cpp
+++ clang/lib/Sema/SemaDeclAttr.cpp
@@ -3953,6 +3953,13 @@
 return;
 
   uint64_t AlignVal = Alignment.getZExtValue();
+  // 16 byte ByVal alignment not due to a vector member is not honoured by XL
+  // on AIX. Emit a warning here that users are generating binary incompatible
+  // code to be safe.
+  if (const auto *FD = dyn_cast(D)) {
+if (Context.getTargetInfo().getTriple().isOSAIX() && AlignVal >= 16)
+  Diag(AttrLoc, diag::warn_not_xl_compatible) << E->getSourceRange();
+  }
 
   // C++11 [dcl.align]p2:
   //   -- if the constant expression evaluates to zero, the alignment
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -3254,6 +3254,10 @@
 : Warning<"requested alignment must be %0 bytes or smaller; maximum "
   "alignment assumed">,
   InGroup>;
+def warn_not_xl_compatible
+: Warning<"requesting an alignment of arguments 16 bytes or greater is not"
+  " binary compatible with AIX XL 16.1 and older">,
+  InGroup;
 def warn_redeclaration_without_attribute_prev_attribute_ignored : Warning<
   "%q0 redeclared without %1 attribute: previous %1 ignored">,
   InGroup;
Index: clang/include/clang/Basic/DiagnosticGroups.td
===
--- clang/include/clang/Basic/DiagnosticGroups.td
+++ clang/include/clang/Basic/DiagnosticGroups.td
@@ -1083,6 +1083,9 @@
 // A warning group for warnings about code that clang accepts but gcc doesn't.
 def GccCompat : DiagGroup<"gcc-compat">;
 
+// A warning group for warnings about code that may be incompatible on AIX.
+def AIXCompat : DiagGroup<"aix-compat">;
+
 // Warnings for Microsoft extensions.
 def MicrosoftCharize : DiagGroup<"microsoft-charize">;
 def MicrosoftDrectveSection : DiagGroup<"microsoft-drectve-section">;


Index: clang/test/Sema/aix-attr-align.c
===
--- /dev/null
+++ clang/test/Sema/aix-attr-align.c
@@ -0,0 +1,17 @@
+// RUN: %clang_cc1 -triple powerpc64-ibm-aix-xcoff -verify -fsyntax-only %s
+// RUN: %clang_cc1 -triple powerpc-ibm-aix-xcoff -verify -fsyntax-only %s
+
+struct S {
+  int a[8] __attribute__((aligned(8)));  // no-warning
+};
+
+struct T {
+  int a[4] __attribute__((aligned(16))); // expected-warning {{requesting an alignment of arguments 16 bytes or greater is not binary compatible with AIX XL 16.1 and older}}
+};
+
+struct U {
+  int a[2] __attribute__((aligned(32))); // expected-warning {{requesting an alignment of arguments 16 bytes or greater is not binary compatible with AIX XL 16.1 and older}}
+};
+  int a[8] __attribute__((aligned(8)));  // no-warning
+  int b[4] __attribute__((aligned(16))); // no-warning 
+  int c[2] __attribute__((aligned(32))); // no-warning
Index: clang/lib/Sema/SemaDeclAttr.cpp
===
--- clang/lib/Sema/SemaDeclAttr.cpp
+++ clang/lib/Sema/SemaDeclAttr.cpp
@@ -3953,6 +3953,13 @@
 return;
 
   uint64_t AlignVal = Alignment.getZExtValue();
+  // 16 byte ByVal alignment not due to a vector member is not honoured by XL
+  // on AIX. Emit a warning here that users are generating binary incompatible
+  // code to be safe.
+  if (const auto *FD = dyn_cast(D)) {
+if (Context.getTargetInfo().getTriple(

[PATCH] D105194: [PowerPC] Add PowerPC cmpb builtin and emit target indepedent code for XL compatibility

[Nemanja Ivanovic via Phabricator via cfe-commits]

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

Also if we are going to be emitting this complex sequence, in the description 
of the patch, point out the test case that shows the back end handles this and 
emits a single instruction.




Comment at: clang/lib/CodeGen/CGBuiltin.cpp:15081
   }
+  case PPC::BI__builtin_ppc_cmpb: {
+llvm::Type *Ty = Ops[0]->getType();

I find it rather surprising that we are emitting this complex sequence for this 
builtin. Perhaps there is a good reason for doing so, but at the very least, 
this requires a thorough explanation in a comment.

One additional concern I have with this is that if some transformation proves 
that some portion of this is unused (perhaps using `DemandedBits` analysis), it 
may optimize out a portion of this, thereby making the sequence emit a whole 
bunch of xor's, or's, rotates, etc.

For example:
```
...
unsigned long long A = __builtin_ppc_cmpb(B, C);
return A & 0xFF00FF00FF00FF;
```
It is entirely possible that the optimizer will get rid of some of the produced 
instructions and then the back end won't be able to emit a single `cmpb` but 
will have to emit a whole bunch of scalar instructions.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105194

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

[PATCH] D105873: Fix utils/update_cc_test_checks/check-globals.test on stand-alone builds

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

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

Thanks for the fix.  LGTM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105873

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

[PATCH] D103096: [analyzer] Implement cast for ranges of symbolic integers.

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov added inline comments.



Comment at: 
clang/include/clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h:293-296
+SymbolRef Sym = Operand;
+while (isa(Sym))
+  Sym = cast(Sym)->Operand;
+return Sym;

vsavchenko wrote:
> ASDenysPetrov wrote:
> > vsavchenko wrote:
> > > 
> > Do you think the recursive call is better than the loop? But, I guess, I 
> > see your point. You option could be safer if we had another implementation 
> > of the virtual method. Or you think such alike cast symbol is possible in 
> > the future? Well, for now `ignoreCasts` doesn't make sense to any other 
> > `Expr` successors.
> Oh, wait, why is it even virtual?  I don't think that it should be virtual.
> Are similar functions in `Expr` virtual?
> And I think that this implementation should live in `SymExpr` directly.  Then 
> it would look like:
> ```
> if (const SymbolCast *ThisAsCast = dyn_cast(this)) {
>   return ThisAsCast->ignoreCasts();
> }
> return this;
> ```
Yes, `SymExpr` is an abstract class. And because of limitations and dependency 
of inheritance we are not able to know the implementaion of `SymbolCast`. 
Unfortunately, this is not a CRTP.



Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:1203
+if (!Opts.ShouldSupportSymbolicIntegerCasts)
+  return VisitSymExpr(Sym);
+

vsavchenko wrote:
> ASDenysPetrov wrote:
> > vsavchenko wrote:
> > > Why do you use `VisitSymExpr` here?  You want to interrupt all `Visits 
> > > or... I'm not sure I fully understand.
> > Here we want to delegate the reasoning to another handler as we don't 
> > support non-integral cast yet.
> You are not delegating it here.  `Visit` includes a runtime dispatch that 
> calls a correct `VisitTYPE` method.  Here you call `VisitSymExpr` directly, 
> which is one of the `VisitTYPE` methods.  No dispatch will happen, and since 
> we use `VisitSymExpr` as the last resort (it's the most base class, if we got 
> there, we don't actually support the expression), you interrupt the `Visit` 
> and go directly to "the last resort".
> 
> See the problem now?
OK. I reject this idea before. If we call `Visit` inside `VisitSymbolCast`, we 
will go into recursive loop, because it will return us back to 
`VisitSymbolCast` as we have passed `Sym` as is. (This is theoretically, I 
didn't check in practice.) Or I'm missing smth?
I choosed `VisitSymExpr` here because all kinds of `SymbolCast` were previously 
handled here. So I decided to pass all unsupproted forms of casts there.



Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:1241
+  // Find the first constraint and exit the loop.
+  RSPtr = getConstraint(State, S);
+}

vsavchenko wrote:
> ASDenysPetrov wrote:
> > vsavchenko wrote:
> > > Why do you get associated constraint directly without consulting with 
> > > what `SymbolRangeInferrer` can tell you about it?
> > What do you mean? I didn't get. Could you give an example?
> `getConstraint` returns whatever constraint we have stored directly in the 
> constraint map.  That's the main source of information for ranges, but not 
> the only one.
> 
> Here is the  of things that you skip, when you do `getConstraint` here:
>   * we can understand that something is equality/disequality check and find 
> the corresponding info in Equivalence Classes data structure
>   * we can see that the expression has the form `A - B` and we can find 
> constraint for `B - A`
>   * we can see that the expression is comparison `A op B` and check what 
> other comparison info we have on `A` and `B` (your own change)
>   * we can see that the expression is of form `A op B` and check if we know 
> something about `A` and `B`, and produce a reasonable constraint out of this 
> information
> 
> In order to use the right information, you should use `infer` that will 
> actually do all other things as well.  That's how `SymbolRangeInferrer` is 
> designed, to be **recursive**.
> 
> Speaking of **recursiveness**.  All these loops and manually checking for 
> types of the cast's operand is against this pattern.  Recursive visitors 
> should call `Visit` for children nodes (like `RecursiveASTVisitor`).  In 
> other words, if `f(x)` is a visit function, it should be defined like this:
> ```
> f(x) = g(f(x->operand_1), f(x->operand_2), ... , f(x->operand_N))
> ```
> or if we talk about your case specifically:
> ```
> f(x: SymbolCast) = h(f(x->Operand))
> ```
> and the `h` function should transform the range set returned by 
> `f(x->Operand)` into a range set appropriate for `x`.
> 
> NOTE: `h` can also intersect different ranges
Thank you for useful notes!  I'll take them into account.


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

https://reviews.llvm.org/D103096

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

[PATCH] D105360: [PowerPC] Fix popcntb XL Compat Builtin for 32bit

[Amy Kwan via Phabricator via cfe-commits]

amyk accepted this revision.
amyk added a comment.

Minor indentation comments but LGTM.




Comment at: clang/test/CodeGen/builtins-ppc-xlcompat-sync.c:3
 // RUN: %clang_cc1 -triple powerpc64-unknown-unknown \
-// RUN:-emit-llvm %s -o -  -target-cpu pwr8 | FileCheck %s
+// RUN: -emit-llvm %s -o - -target-cpu pwr7 | FileCheck %s
 // RUN: %clang_cc1 -triple powerpc64le-unknown-unknown \





Comment at: llvm/test/CodeGen/PowerPC/builtins-ppc-xlcompat-msync.ll:3
+; RUN: llc -verify-machineinstrs -mtriple=powerpc64-unknown-unknown \
+; RUN: --ppc-asm-full-reg-names -mattr=+msync -mcpu=pwr7 < %s | FileCheck %s
+; RUN: llc -verify-machineinstrs -mtriple=powerpc64le-unknown-unknown \

Minor indentation nit.



Comment at: llvm/test/CodeGen/PowerPC/builtins-ppc-xlcompat-sync-32.ll:3
+; RUN: llc -verify-machineinstrs -mtriple=powerpc-unknown-aix \
+; RUN: --ppc-asm-full-reg-names -mcpu=pwr7 < %s | FileCheck %s
+

Minor indentation nit.



Comment at: llvm/test/CodeGen/PowerPC/builtins-ppc-xlcompat-sync-64.ll:3
+; RUN: llc -verify-machineinstrs -mtriple=powerpc64-unknown-unknown \
+; RUN: --ppc-asm-full-reg-names -mcpu=pwr7 < %s | FileCheck %s
+; RUN: llc -verify-machineinstrs -mtriple=powerpc64le-unknown-unknown \

Minor indentation nit.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105360

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

[PATCH] D105892: [NFC] Silence build warning by placing parentheses around condition

[Anton Zabaznov via Phabricator via cfe-commits]

azabaznov updated this revision to Diff 358265.
azabaznov added a comment.

Add comment for C++ for OpenCL, add variable to check support for OpenCL C


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105892

Files:
  clang/lib/Sema/SemaDeclAttr.cpp


Index: clang/lib/Sema/SemaDeclAttr.cpp
===
--- clang/lib/Sema/SemaDeclAttr.cpp
+++ clang/lib/Sema/SemaDeclAttr.cpp
@@ -7402,14 +7402,17 @@
   // OpenCL v3.0 s6.8 - For OpenCL C 2.0, or with the
   // __opencl_c_read_write_images feature, image objects specified as arguments
   // to a kernel can additionally be declared to be read-write.
+  // C++ for OpenCL inherits rule from OpenCL C v2.0.
   if (const auto *PDecl = dyn_cast(D)) {
 const Type *DeclTy = PDecl->getType().getCanonicalType().getTypePtr();
 if (AL.getAttrName()->getName().find("read_write") != StringRef::npos) {
-  if (((!S.getLangOpts().OpenCLCPlusPlus &&
-(S.getLangOpts().OpenCLVersion < 200)) ||
-   (S.getLangOpts().OpenCLVersion == 300 &&
-!S.getOpenCLOptions().isSupported("__opencl_c_read_write_images",
-  S.getLangOpts( ||
+  bool ReadWriteImagesUnsupportedForOCLC =
+  (S.getLangOpts().OpenCLVersion < 200) ||
+  (S.getLangOpts().OpenCLVersion == 300 &&
+   !S.getOpenCLOptions().isSupported("__opencl_c_read_write_images",
+ S.getLangOpts()));
+  if ((!S.getLangOpts().OpenCLCPlusPlus &&
+   ReadWriteImagesUnsupportedForOCLC) ||
   DeclTy->isPipeType()) {
 S.Diag(AL.getLoc(), diag::err_opencl_invalid_read_write)
 << AL << PDecl->getType() << DeclTy->isImageType();


Index: clang/lib/Sema/SemaDeclAttr.cpp
===
--- clang/lib/Sema/SemaDeclAttr.cpp
+++ clang/lib/Sema/SemaDeclAttr.cpp
@@ -7402,14 +7402,17 @@
   // OpenCL v3.0 s6.8 - For OpenCL C 2.0, or with the
   // __opencl_c_read_write_images feature, image objects specified as arguments
   // to a kernel can additionally be declared to be read-write.
+  // C++ for OpenCL inherits rule from OpenCL C v2.0.
   if (const auto *PDecl = dyn_cast(D)) {
 const Type *DeclTy = PDecl->getType().getCanonicalType().getTypePtr();
 if (AL.getAttrName()->getName().find("read_write") != StringRef::npos) {
-  if (((!S.getLangOpts().OpenCLCPlusPlus &&
-(S.getLangOpts().OpenCLVersion < 200)) ||
-   (S.getLangOpts().OpenCLVersion == 300 &&
-!S.getOpenCLOptions().isSupported("__opencl_c_read_write_images",
-  S.getLangOpts( ||
+  bool ReadWriteImagesUnsupportedForOCLC =
+  (S.getLangOpts().OpenCLVersion < 200) ||
+  (S.getLangOpts().OpenCLVersion == 300 &&
+   !S.getOpenCLOptions().isSupported("__opencl_c_read_write_images",
+ S.getLangOpts()));
+  if ((!S.getLangOpts().OpenCLCPlusPlus &&
+   ReadWriteImagesUnsupportedForOCLC) ||
   DeclTy->isPipeType()) {
 S.Diag(AL.getLoc(), diag::err_opencl_invalid_read_write)
 << AL << PDecl->getType() << DeclTy->isImageType();
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D104744: [PowerPC] Add PowerPC rotate related builtins and emit target independent code for XL compatibility

[Nemanja Ivanovic via Phabricator via cfe-commits]

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



Comment at: clang/include/clang/Sema/Sema.h:12559
   bool SemaBuiltinOSLogFormat(CallExpr *TheCall);
+  bool CheckPPCisRunOfOnes(CallExpr *TheCall, unsigned ArgNum);
+  bool CheckPPC64isRunOfOnes(CallExpr *TheCall, unsigned ArgNum);

I don't think these names adequately describe what the check is. We are not 
checking if `PPC/PPC64` is a run of ones. In fact, the concept of a contiguous 
mask is in no way specific to PPC so we don't really need to differentiate this 
as a PPC-specific check.
Also, the convention seems to be to prefix all names with `Sema`. Let's not 
part with that convention.
Perhaps `SemaValueIsRunOfOnes()` or `SemaArgIsRunOfOnes()`.



Comment at: clang/lib/Sema/SemaChecking.cpp:3299
+  unsigned Val = Result.getExtValue();
+  if (!Val)
+return Diag(TheCall->getBeginLoc(),

Isn't a zero technically a contiguous run of ones (of length zero)?



Comment at: clang/lib/Sema/SemaChecking.cpp:3429
SemaBuiltinConstantArgRange(TheCall, 0, 0, 1);
+  case PPC::BI__builtin_ppc_rlwnm:
+return SemaBuiltinConstantArg(TheCall, 1, Result) ||

Please describe in a comment why we need to ensure that the argument is a 
contiguous mask.



Comment at: clang/lib/Sema/SemaChecking.cpp:3430
+  case PPC::BI__builtin_ppc_rlwnm:
+return SemaBuiltinConstantArg(TheCall, 1, Result) ||
+   CheckPPCisRunOfOnes(TheCall, 2);

We seem to populate the `Result` here. If we already have the `Result` (i.e. 
the constant value) can't we just simply check if that is a run of ones? Then 
we can greatly simplify `CheckPPCisRunOfOnes()` and `CheckPPC64isRunOfOnes()` 
(and we may not actually need two functions since `Result` will be an `APSInt` 
that knows how wide it is. We could presumably just have:
`bool SemaValueIsRunOfOnes(const APSInt &Val, unsigned Width);`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104744

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

[PATCH] D103750: [analyzer] Handle std::make_unique for SmartPtrModeling

[Deep Majumder via Phabricator via cfe-commits]

RedDocMD updated this revision to Diff 358267.
RedDocMD added a comment.

Fixing up tests


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103750

Files:
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h
  clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp
  clang/lib/StaticAnalyzer/Core/SValBuilder.cpp
  clang/test/Analysis/Inputs/system-header-simulator-cxx.h
  clang/test/Analysis/smart-ptr-text-output.cpp

Index: clang/test/Analysis/smart-ptr-text-output.cpp
===
--- clang/test/Analysis/smart-ptr-text-output.cpp
+++ clang/test/Analysis/smart-ptr-text-output.cpp
@@ -1,10 +1,17 @@
 // RUN: %clang_analyze_cc1\
-// RUN:  -analyzer-checker=core,cplusplus.Move,alpha.cplusplus.SmartPtr\
+// RUN:  -analyzer-checker=core,cplusplus.Move,alpha.cplusplus.SmartPtr,debug.ExprInspection\
+// RUN:  -analyzer-config cplusplus.SmartPtrModeling:ModelSmartPtrDereference=true\
+// RUN:  -analyzer-output=text -std=c++20 %s -verify=expected
+
+// RUN: %clang_analyze_cc1\
+// RUN:  -analyzer-checker=core,cplusplus.Move,alpha.cplusplus.SmartPtr,debug.ExprInspection\
 // RUN:  -analyzer-config cplusplus.SmartPtrModeling:ModelSmartPtrDereference=true\
 // RUN:  -analyzer-output=text -std=c++11 %s -verify=expected
 
 #include "Inputs/system-header-simulator-cxx.h"
 
+void clang_analyzer_eval(bool);
+
 class A {
 public:
   A(){};
@@ -313,3 +320,55 @@
 // expected-note@-1{{Dereference of null smart pointer 'P'}}
   }
 }
+
+void makeUniqueReturnsNonNullUniquePtr() {
+  auto P = std::make_unique();
+  if (!P) {   // expected-note {{Taking false branch}}
+P->foo(); // should have no warning here, path is impossible
+  }
+  P.reset(); // expected-note {{Smart pointer 'P' reset using a null value}}
+  // Now P is null
+  if (!P) {
+// expected-note@-1 {{Taking true branch}}
+P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
+// expected-note@-1{{Dereference of null smart pointer 'P'}}
+  }
+}
+
+#if __cplusplus >= 202002L
+
+void makeUniqueForOverwriteReturnsNullUniquePtr() {
+  auto P = std::make_unique_for_overwrite();
+  if (!P) {   // expected-note {{Taking false branch}}
+P->foo(); // should have no warning here, path is impossible
+  }
+  P.reset(); // expected-note {{Smart pointer 'P' reset using a null value}}
+  // Now P is null
+  if (!P) {
+// expected-note@-1 {{Taking true branch}}
+P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
+// expected-note@-1{{Dereference of null smart pointer 'P'}}
+  }
+}
+
+#endif
+
+struct G {
+  int *p;
+  G(int *p): p(p) {}
+  ~G() { *p = 0; }
+};
+
+void foo() {
+  int x = 1;
+  {
+auto P = std::make_unique(&x);
+clang_analyzer_eval(*P->p == 1); // expected-warning {{TRUE}}
+// expected-note@-1 {{Assuming the condition is false}}
+// expected-note@-2 {{FALSE}}
+// expected-note@-3 {{Assuming the condition is true}}
+// expected-note@-4 {{TRUE}}
+  }
+  clang_analyzer_eval(x == 0); // expected-warning {{FALSE}}
+  // expected-note@-1 {{FALSE}}
+}
Index: clang/test/Analysis/Inputs/system-header-simulator-cxx.h
===
--- clang/test/Analysis/Inputs/system-header-simulator-cxx.h
+++ clang/test/Analysis/Inputs/system-header-simulator-cxx.h
@@ -978,6 +978,17 @@
 void swap(unique_ptr &x, unique_ptr &y) noexcept {
   x.swap(y);
 }
+
+template 
+unique_ptr make_unique(Args &&...args);
+
+#if __cplusplus >= 202002L
+
+template 
+unique_ptr make_unique_for_overwrite();
+
+#endif
+
 } // namespace std
 #endif
 
Index: clang/lib/StaticAnalyzer/Core/SValBuilder.cpp
===
--- clang/lib/StaticAnalyzer/Core/SValBuilder.cpp
+++ clang/lib/StaticAnalyzer/Core/SValBuilder.cpp
@@ -192,12 +192,19 @@
   const LocationContext *LCtx,
   unsigned VisitCount) {
   QualType T = E->getType();
-  assert(Loc::isLocType(T));
-  assert(SymbolManager::canSymbolicate(T));
-  if (T->isNullPtrType())
-return makeZeroVal(T);
+  return getConjuredHeapSymbolVal(E, LCtx, T, VisitCount);
+}
+
+DefinedOrUnknownSVal
+SValBuilder::getConjuredHeapSymbolVal(const Expr *E,
+  const LocationContext *LCtx,
+  QualType type, unsigned VisitCount) {
+  assert(Loc::isLocType(type));
+  assert(SymbolManager::canSymbolicate(type));
+  if (type->isNullPtrType())
+return makeZeroVal(type);
 
-  SymbolRef sym = SymMgr.conjureSymbol(E, LCtx, T, VisitCount);
+  SymbolRef sym = SymMgr.conjureSymbol(E, LCtx, type, VisitCount);
   return loc::MemRegionVal(MemMgr.getSymbolicHeapRegion(sym));
 }
 
Index: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp
=

[PATCH] D103096: [analyzer] Implement cast for ranges of symbolic integers.

[Valeriy Savchenko via Phabricator via cfe-commits]

vsavchenko added inline comments.



Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:1203
+if (!Opts.ShouldSupportSymbolicIntegerCasts)
+  return VisitSymExpr(Sym);
+

ASDenysPetrov wrote:
> vsavchenko wrote:
> > ASDenysPetrov wrote:
> > > vsavchenko wrote:
> > > > Why do you use `VisitSymExpr` here?  You want to interrupt all `Visits 
> > > > or... I'm not sure I fully understand.
> > > Here we want to delegate the reasoning to another handler as we don't 
> > > support non-integral cast yet.
> > You are not delegating it here.  `Visit` includes a runtime dispatch that 
> > calls a correct `VisitTYPE` method.  Here you call `VisitSymExpr` directly, 
> > which is one of the `VisitTYPE` methods.  No dispatch will happen, and 
> > since we use `VisitSymExpr` as the last resort (it's the most base class, 
> > if we got there, we don't actually support the expression), you interrupt 
> > the `Visit` and go directly to "the last resort".
> > 
> > See the problem now?
> OK. I reject this idea before. If we call `Visit` inside `VisitSymbolCast`, 
> we will go into recursive loop, because it will return us back to 
> `VisitSymbolCast` as we have passed `Sym` as is. (This is theoretically, I 
> didn't check in practice.) Or I'm missing smth?
> I choosed `VisitSymExpr` here because all kinds of `SymbolCast` were 
> previously handled here. So I decided to pass all unsupproted forms of casts 
> there.
Did I suggest to `Visit(Sym)`?  Of course it is going to end up in a loop!  
Why isn't it `Visit(Sym->getOperand())` here?  Before we started producing 
casts, casts were transparent.  This logic would fit perfectly with that.



Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:1216
+  if (!T->isIntegralOrEnumerationType())
+return VisitSymExpr(Sym);
+

vsavchenko wrote:
> Same goes here.
And here, since we couldn't really reason about it, we usually return 
`infer(T)`.


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

https://reviews.llvm.org/D103096

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

[PATCH] D103750: [analyzer] Handle std::make_unique for SmartPtrModeling

[Deep Majumder via Phabricator via cfe-commits]

RedDocMD added a comment.

Is the syntax of specifying expected notes and warnings documented somewhere? I 
could not find the note-specific syntax.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103750

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

[PATCH] D105892: [OpenCL] Add verbosity when checking support of read_write images

[Anastasia Stulova via Phabricator via cfe-commits]

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

LGTM! Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105892

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

[PATCH] D105892: [OpenCL] Add verbosity when checking support of read_write images

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added a comment.

In D105892#2873951 , @aaron.ballman 
wrote:

> Note: https://reviews.llvm.org/D105890 that was just landed to touch the same 
> area.

Yeah, I think it fixed the actual warning but we can still simplify the 
condition.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105892

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

[clang] 1e03c37 - Prepare Compiler-RT for GnuInstallDirs, matching libcxx, document all

[John Ericson via cfe-commits]

Author: John Ericson
Date: 2021-07-13T15:21:41Z
New Revision: 1e03c37b97b6176a60404d84665c40321f4e33a4

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

LOG: Prepare Compiler-RT for GnuInstallDirs, matching libcxx, document all

This is a second attempt at D101497, which landed as
9a9bc76c0eb72f0f2732c729a460abbd5239c2e3 but had to be reverted in
8cf7ddbdd4e5af966a369e170c73250f2e3920e7.

This issue was that in the case that `COMPILER_RT_INSTALL_PATH` is
empty, expressions like "${COMPILER_RT_INSTALL_PATH}/bin" evaluated to
"/bin" not "bin" as intended and as was originally.

One solution is to make `COMPILER_RT_INSTALL_PATH` always non-empty,
defaulting it to `CMAKE_INSTALL_PREFIX`. D99636 adopted that approach.
But, I think it is more ergonomic to allow those project-specific paths
to be relative the global ones. Also, making install paths absolute by
default inhibits the proper behavior of functions like
`GNUInstallDirs_get_absolute_install_dir` which make relative install
paths absolute in a more complicated way.

Given all this, I will define a function like the one asked for in
https://gitlab.kitware.com/cmake/cmake/-/issues/19568 (and needed for a
similar use-case).

---

Original message:

Instead of using `COMPILER_RT_INSTALL_PATH` through the CMake for
complier-rt, just use it to define variables for the subdirs which
themselves are used.

This preserves compatibility, but later on we might consider getting rid
of `COMPILER_RT_INSTALL_PATH` and just changing the defaults for the
subdir variables directly.

---

There was a seaming bug where the (non-Apple) per-target libdir was
`${target}` not `lib/${target}`. I suspect that has to do with the docs
on `COMPILER_RT_INSTALL_PATH` saying was the library dir when that's no
longer true, so I just went ahead and fixed it, allowing me to define
fewer and more sensible variables.

That last part should be the only behavior changes; everything else
should be a pure refactoring.

---

I added some documentation of these variables too. In particular, I
wanted to highlight the gotcha where `-DSomeCachePath=...` without the
`:PATH` will lead CMake to make the path absolute. See [1] for
discussion of the problem, and [2] for the brief official documentation
they added as a result.

[1]: https://cmake.org/pipermail/cmake/2015-March/060204.html

[2]: https://cmake.org/cmake/help/latest/manual/cmake.1.html#options

In 38b2dec37ee735d5409148e71ecba278caf0f969 the problem was somewhat
misidentified and so `:STRING` was used, but `:PATH` is better as it
sets the correct type from the get-go.

---

D99484 is the main thrust of the `GnuInstallDirs` work. Once this lands,
it should be feasible to follow both of these up with a simple patch for
compiler-rt analogous to the one for libcxx.

Reviewed By: phosek, #libc_abi, #libunwind

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

Added: 
compiler-rt/docs/BuildingCompilerRT.rst

Modified: 
clang/runtime/CMakeLists.txt
compiler-rt/cmake/Modules/AddCompilerRT.cmake
compiler-rt/cmake/Modules/CompilerRTDarwinUtils.cmake
compiler-rt/cmake/Modules/CompilerRTUtils.cmake
compiler-rt/cmake/base-config-ix.cmake
compiler-rt/include/CMakeLists.txt
compiler-rt/lib/dfsan/CMakeLists.txt
libcxx/CMakeLists.txt
libcxx/docs/BuildingLibcxx.rst
libcxxabi/CMakeLists.txt
libunwind/CMakeLists.txt
libunwind/docs/BuildingLibunwind.rst

Removed: 




diff  --git a/clang/runtime/CMakeLists.txt b/clang/runtime/CMakeLists.txt
index 1716c53b031ef..61b1c60bf590b 100644
--- a/clang/runtime/CMakeLists.txt
+++ b/clang/runtime/CMakeLists.txt
@@ -82,7 +82,7 @@ if(LLVM_BUILD_EXTERNAL_COMPILER_RT AND EXISTS 
${COMPILER_RT_SRC_ROOT}/)
-DLLVM_LIT_ARGS=${LLVM_LIT_ARGS}

-DCOMPILER_RT_OUTPUT_DIR=${LLVM_LIBRARY_OUTPUT_INTDIR}/clang/${CLANG_VERSION}
-DCOMPILER_RT_EXEC_OUTPUT_DIR=${LLVM_RUNTIME_OUTPUT_INTDIR}
-   
-DCOMPILER_RT_INSTALL_PATH:STRING=lib${LLVM_LIBDIR_SUFFIX}/clang/${CLANG_VERSION}
+   
-DCOMPILER_RT_INSTALL_PATH:PATH=lib${LLVM_LIBDIR_SUFFIX}/clang/${CLANG_VERSION}
-DCOMPILER_RT_INCLUDE_TESTS=${LLVM_INCLUDE_TESTS}
-DCMAKE_INSTALL_PREFIX=${CMAKE_INSTALL_PREFIX}
-DLLVM_LIBDIR_SUFFIX=${LLVM_LIBDIR_SUFFIX}

diff  --git a/compiler-rt/cmake/Modules/AddCompilerRT.cmake 
b/compiler-rt/cmake/Modules/AddCompilerRT.cmake
index 1e9e7c58664b6..bc69ec95c4195 100644
--- a/compiler-rt/cmake/Modules/AddCompilerRT.cmake
+++ b/compiler-rt/cmake/Modules/AddCompilerRT.cmake
@@ -371,7 +371,7 @@ function(add_compiler_rt_runtime name type)
 add_custom_command(TARGET ${libname}
   POST_BUILD  
   COMMAND codesign --sign - $
-  WORKING_DIRECTORY ${C

[PATCH] D102107: [OpenMP] Codegen aggregate for outlined function captures

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert added a comment.

In D102107#2867693 , @ABataev wrote:

> In D102107#2867670 , @josemonsalve2 
> wrote:
>
>> In D102107#2867417 , @ABataev 
>> wrote:
>>
>>> In D102107#2867382 , @jdoerfert 
>>> wrote:
>>>
 In D102107#2832740 , @ABataev 
 wrote:

> In D102107#2832286 , @jdoerfert 
> wrote:
>
>> In D102107#2824581 , @ABataev 
>> wrote:
>>
>>> In D102107#2823706 , 
>>> @jdoerfert wrote:
>>>
 In D102107#2821976 , 
 @ABataev wrote:

> We used this kind of codegen initially but later found out that it 
> causes a large overhead when gathering pointers into a record. What 
> about hybrid scheme where the first args are passed as arguments and 
> others (if any) are gathered into a record?

 I'm confused, maybe I misunderstand the problem. The parallel function 
 arguments need to go from the main thread to the workers somehow, I 
 don't see how this is done w/o a record. This patch makes it explicit 
 though.
>>>
>>> Pass it in a record for workers only? And use a hybrid scheme for all 
>>> other parallel regions.
>>
>> I still do not follow. What does it mean for workers only? What is a 
>> hybrid scheme? And, probably most importantly, how would we not 
>> eventually put everything into a record anyway?
>
> On the host you don’t need to put everything into a record, especially 
> for small parallel regions. Pass some first args in registers and only 
> the remaining args gather into the record. For workers just pass all args 
> in the record.

 Could you please respond to my question so we make progress here. We 
 *always* have to pass things in a record, do you agree?
>>>
>>> On the GPU device, yes. And I'm absolutely fine with packing args for the 
>>> GPU device. But the patch packs the args not only for the GPU devices but 
>>> also for the host and other devices which may not require 
>>> packing/unpacking. For such devices/host better to avoid packing/unpacking 
>>> as it introduces overhead in many cases.
>>
>> Hi Alexey,
>>
>> Wouldn't you always need to pack to pass the arguments to the outlined 
>> function? What is the benefit of avoiding packing the arguments in the 
>> runtime call, if then you have to pack them for the outlined function?
>>
>> I would really appreciate an example, since I am just getting an 
>> understanding of OpenMP in LLVM.
>>
>> Thanks!
>
> Hi, generally speaking, no, you don't need to pack them. Initially, we 
> packed/unpacked args, but then decided not to do it.
> Here is an example:
>
>   int a, b;
>   #pragma omp parallel
>printf("%d %d\n", a, b);
>
> What we generate currently is something like this:
>
>   %a = alloca i32
>   %b = alloca i32
>   call __kmpc_fork_call(..., @outlined, %a, %b)
>   ...
>internal @outlined(i32 *%a, i32 *%b) {
>printf();
>}
>
> `__kmpc_fork_call` inside calls `@outlined` function with the passed args.

While on the user facing side this does not pack the arguments, we still do, 
and that is the point. In the runtime `__kmp_fork_call` we do

  for (i = argc - 1; i >= 0; --i)
 *argv++ = va_arg(kmp_va_deref(ap), void *);  

which packs the variadic arguments into a buffer. So eventually we have to walk 
them and store them into a consecutive buffer. What
happens before is that we pass some of them in registers but at the end of the 
day we put them in memory. After all, that is what
`extern int __kmp_invoke_microtask(microtask_t pkfn, int gtid, int npr, int 
argc, void *argv[], ...`
expects.

If you believe we do not pack/unpack on the host, please walk me through that. 
As far as I can tell, the "user facing side" might
look like variadic calls all the way but that is not what is happening in the 
runtime. Thus, there is no apparent reason to complicate
the scheme. I'm also happy if you have timing results that indicate otherwise.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102107

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

[PATCH] D105881: [flang][driver] Switch to `BoolFOption` for boolean options

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 added inline comments.



Comment at: clang/include/clang/Driver/Options.td:246
 
+class EmptyKPM : KeyPathAndMacro<"", "", ""> {}
 class DiagnosticOpts

awarzynski wrote:
> jansvoboda11 wrote:
> > awarzynski wrote:
> > > @jansvoboda11 , is this the right approach here? I'd like use 
> > > `BoolFOption` in Flang, but we don't need `KeyPathAndMacro` just yet. We 
> > > may do in the future.
> > There was an `EmptyKPM` class present in the code before, but only as an 
> > implementation detail of the marshalling system. People started using it 
> > with `BoolFOption` to conveniently declare pairs Clang driver flags. I'm 
> > not a fan since the whole `BooFOption` is designed around the keypath and 
> > its marshalling. Putting a "null" keypath in `BoolFOption` and then talking 
> > about its defaults and the effect of each flag seems highly unintuitive to 
> > me and it's essentially dead code. I'd prefer a different approach in this 
> > patch unless you have plans to switch to the marshalling infrastructure and 
> > remove `EmptyKPM` in the short term.
> > 
> > Would something similar to `OptInFFlag` and `OptOutFFlag` work for you? You 
> > could generalize it (remove `Flags<[CC1Option]>`), make use of it for 
> > Flang's options directly and forward the current `Opt{In,Out}FFlag` to the 
> > generalization while specifying the `CC1Option`. WDYT?
> Thanks for the quick feedback! Initially when I saw `CC1Option` in 
> `CC1Option`/`OptOutFFlag`, I immediately decided to explore alternatives. But 
> I agree with everything that you said and am happy to generalize these 
> multiclasses (instead of adding `EmptyKPM`). I don't see why it wouldn't work 
> for what we currently need in Flang. I should be able to update this patch 
> tomorrow.
> 
> Btw, what's the key benefit of the marshaling infra? Is that for being able 
> to restore the original compiler invocation from instances of 
> `CompilerInvocation`? We are still busy with other things, but definitely 
> don't want Flang to miss out here.
Cool, happy to help.

We're using marshalling for "implicitly discovered, explicitly built Clang 
modules" via `clang-scan-deps`. We're able to change the `CompilerInvocation` 
of the original TU for the purpose of explicit build of the discovered modules, 
serialize it to `clang -cc1` command-line and report it to the build system.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105881

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

[PATCH] D105898: [OpenMP] Rework OpenMP remarks

[Joseph Huber via Phabricator via cfe-commits]

jhuber6 created this revision.
jhuber6 added a reviewer: jdoerfert.
Herald added subscribers: ormris, okura, kuter, guansong, hiraditya, yaxunl.
jhuber6 requested review of this revision.
Herald added a reviewer: sstefan1.
Herald added subscribers: llvm-commits, cfe-commits, bbn, sstefan1.
Herald added a reviewer: baziotis.
Herald added projects: clang, LLVM.

This patch rewrites and reworks a few of the existing remarks to make the mmore
concise and consistent prior to writing the documentation for them.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D105898

Files:
  clang/test/OpenMP/remarks_parallel_in_multiple_target_state_machines.c
  clang/test/OpenMP/remarks_parallel_in_target_state_machine.c
  llvm/lib/Transforms/IPO/AttributorAttributes.cpp
  llvm/lib/Transforms/IPO/OpenMPOpt.cpp
  llvm/test/Transforms/OpenMP/custom_state_machines_remarks.ll
  llvm/test/Transforms/OpenMP/deduplication_remarks.ll
  llvm/test/Transforms/OpenMP/globalization_remarks.ll
  llvm/test/Transforms/OpenMP/parallel_deletion_remarks.ll
  llvm/test/Transforms/OpenMP/remove_globalization.ll
  llvm/test/Transforms/OpenMP/spmdization_remarks.ll

Index: llvm/test/Transforms/OpenMP/spmdization_remarks.ll
===
--- llvm/test/Transforms/OpenMP/spmdization_remarks.ll
+++ llvm/test/Transforms/OpenMP/spmdization_remarks.ll
@@ -1,12 +1,13 @@
 ; RUN: opt -passes=openmp-opt -pass-remarks=openmp-opt -pass-remarks-missed=openmp-opt -pass-remarks-analysis=openmp-opt -disable-output < %s 2>&1 | FileCheck %s
 target triple = "nvptx64"
 
-; CHECK: remark: llvm/test/Transforms/OpenMP/spmdization_remarks.c:13:5: Kernel will be executed in generic-mode due to this potential side-effect, consider to add `__attribute__((assume("ompx_spmd_amenable")))` to the called function 'unknown'.
-; CHECK: remark: llvm/test/Transforms/OpenMP/spmdization_remarks.c:15:5: Kernel will be executed in generic-mode due to this potential side-effect, consider to add `__attribute__((assume("ompx_spmd_amenable")))` to the called function 'unknown'.
-; CHECK: remark: llvm/test/Transforms/OpenMP/spmdization_remarks.c:11:1: Generic-mode kernel is executed with a customized state machine that requires a fallback [1 known parallel regions, 2 unkown parallel regions] (bad).
-; CHECK: remark: llvm/test/Transforms/OpenMP/spmdization_remarks.c:13:5: State machine fallback caused by this call. If it is a false positive, use `__attribute__((assume("omp_no_openmp")))` (or "omp_no_parallelism").
-; CHECK: remark: llvm/test/Transforms/OpenMP/spmdization_remarks.c:15:5: State machine fallback caused by this call. If it is a false positive, use `__attribute__((assume("omp_no_openmp")))` (or "omp_no_parallelism").
-; CHECK: remark: llvm/test/Transforms/OpenMP/spmdization_remarks.c:20:1: Generic-mode kernel is changed to SPMD-mode.
+; CHECK: remark: llvm/test/Transforms/OpenMP/spmdization_remarks.c:13:5: Value has potential side effects preventing SPMD-mode execution. Add `__attribute__((assume("ompx_spmd_amenable")))` to the called function to override.
+; CHECK: remark: llvm/test/Transforms/OpenMP/spmdization_remarks.c:15:5: Value has potential side effects preventing SPMD-mode execution. Add `__attribute__((assume("ompx_spmd_amenable")))` to the called function to override.
+; CHECK: remark: llvm/test/Transforms/OpenMP/spmdization_remarks.c:11:1: Generic-mode kernel is executed with a customized state machine that requires a fallback [1 known parallel regions, 2 unkown parallel regions].
+; CHECK: remark: llvm/test/Transforms/OpenMP/spmdization_remarks.c:13:5: Call may contain unknown parallel regions. Use `__attribute__((assume("omp_no_openmp")))` to override.
+; CHECK: remark: llvm/test/Transforms/OpenMP/spmdization_remarks.c:15:5: Call may contain unknown parallel regions. Use `__attribute__((assume("omp_no_openmp")))` to override.
+; CHECK: remark: llvm/test/Transforms/OpenMP/spmdization_remarks.c:20:1: Transformed Generic-mode kernel to SPMD-mode.
+
 
 ;; void unknown(void);
 ;; void known(void) {
Index: llvm/test/Transforms/OpenMP/remove_globalization.ll
===
--- llvm/test/Transforms/OpenMP/remove_globalization.ll
+++ llvm/test/Transforms/OpenMP/remove_globalization.ll
@@ -4,7 +4,7 @@
 target datalayout = "e-i64:64-i128:128-v16:16-v32:32-n16:32:64"
 target triple = "nvptx64"
 
-; CHECK-REMARKS: remark: remove_globalization.c:4:2: Could not move globalized variable to the stack as variable is potentially captured in call; mark parameter as `__attribute__((noescape))` to override.
+; CHECK-REMARKS: remark: remove_globalization.c:4:2: Could not move globalized variable to the stack. Variable is potentially captured in call. Mark parameter as `__attribute__((noescape))` to override.
 ; CHECK-REMARKS: remark: remove_globalization.c:2:2: Moving globalized variable to the stack.
 ; CHECK-REMARKS: remark: remove_globalization.c:6:2: Moving global

[clang] 03d8fed - [OpenCL] Add verbosity when checking support of read_write images

[Anton Zabaznov via cfe-commits]

Author: Anton Zabaznov
Date: 2021-07-13T18:47:29+03:00
New Revision: 03d8fed34951bc6e92b36615ec3afe6f36d10de6

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

LOG: [OpenCL] Add verbosity when checking support of read_write images

Parenthesis were fixed incorrectly by D105890

Reviewed By: Anastasia

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

Added: 


Modified: 
clang/lib/Sema/SemaDeclAttr.cpp

Removed: 




diff  --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp
index a5d0597e39c4..3586ad323b8e 100644
--- a/clang/lib/Sema/SemaDeclAttr.cpp
+++ b/clang/lib/Sema/SemaDeclAttr.cpp
@@ -7402,14 +7402,17 @@ static void handleOpenCLAccessAttr(Sema &S, Decl *D, 
const ParsedAttr &AL) {
   // OpenCL v3.0 s6.8 - For OpenCL C 2.0, or with the
   // __opencl_c_read_write_images feature, image objects specified as arguments
   // to a kernel can additionally be declared to be read-write.
+  // C++ for OpenCL inherits rule from OpenCL C v2.0.
   if (const auto *PDecl = dyn_cast(D)) {
 const Type *DeclTy = PDecl->getType().getCanonicalType().getTypePtr();
 if (AL.getAttrName()->getName().find("read_write") != StringRef::npos) {
-  if (((!S.getLangOpts().OpenCLCPlusPlus &&
-(S.getLangOpts().OpenCLVersion < 200)) ||
-   (S.getLangOpts().OpenCLVersion == 300 &&
-!S.getOpenCLOptions().isSupported("__opencl_c_read_write_images",
-  S.getLangOpts( ||
+  bool ReadWriteImagesUnsupportedForOCLC =
+  (S.getLangOpts().OpenCLVersion < 200) ||
+  (S.getLangOpts().OpenCLVersion == 300 &&
+   !S.getOpenCLOptions().isSupported("__opencl_c_read_write_images",
+ S.getLangOpts()));
+  if ((!S.getLangOpts().OpenCLCPlusPlus &&
+   ReadWriteImagesUnsupportedForOCLC) ||
   DeclTy->isPipeType()) {
 S.Diag(AL.getLoc(), diag::err_opencl_invalid_read_write)
 << AL << PDecl->getType() << DeclTy->isImageType();



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

[PATCH] D105892: [OpenCL] Add verbosity when checking support of read_write images

[Anton Zabaznov 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 rG03d8fed34951: [OpenCL] Add verbosity when checking support 
of read_write images (authored by azabaznov).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105892

Files:
  clang/lib/Sema/SemaDeclAttr.cpp


Index: clang/lib/Sema/SemaDeclAttr.cpp
===
--- clang/lib/Sema/SemaDeclAttr.cpp
+++ clang/lib/Sema/SemaDeclAttr.cpp
@@ -7402,14 +7402,17 @@
   // OpenCL v3.0 s6.8 - For OpenCL C 2.0, or with the
   // __opencl_c_read_write_images feature, image objects specified as arguments
   // to a kernel can additionally be declared to be read-write.
+  // C++ for OpenCL inherits rule from OpenCL C v2.0.
   if (const auto *PDecl = dyn_cast(D)) {
 const Type *DeclTy = PDecl->getType().getCanonicalType().getTypePtr();
 if (AL.getAttrName()->getName().find("read_write") != StringRef::npos) {
-  if (((!S.getLangOpts().OpenCLCPlusPlus &&
-(S.getLangOpts().OpenCLVersion < 200)) ||
-   (S.getLangOpts().OpenCLVersion == 300 &&
-!S.getOpenCLOptions().isSupported("__opencl_c_read_write_images",
-  S.getLangOpts( ||
+  bool ReadWriteImagesUnsupportedForOCLC =
+  (S.getLangOpts().OpenCLVersion < 200) ||
+  (S.getLangOpts().OpenCLVersion == 300 &&
+   !S.getOpenCLOptions().isSupported("__opencl_c_read_write_images",
+ S.getLangOpts()));
+  if ((!S.getLangOpts().OpenCLCPlusPlus &&
+   ReadWriteImagesUnsupportedForOCLC) ||
   DeclTy->isPipeType()) {
 S.Diag(AL.getLoc(), diag::err_opencl_invalid_read_write)
 << AL << PDecl->getType() << DeclTy->isImageType();


Index: clang/lib/Sema/SemaDeclAttr.cpp
===
--- clang/lib/Sema/SemaDeclAttr.cpp
+++ clang/lib/Sema/SemaDeclAttr.cpp
@@ -7402,14 +7402,17 @@
   // OpenCL v3.0 s6.8 - For OpenCL C 2.0, or with the
   // __opencl_c_read_write_images feature, image objects specified as arguments
   // to a kernel can additionally be declared to be read-write.
+  // C++ for OpenCL inherits rule from OpenCL C v2.0.
   if (const auto *PDecl = dyn_cast(D)) {
 const Type *DeclTy = PDecl->getType().getCanonicalType().getTypePtr();
 if (AL.getAttrName()->getName().find("read_write") != StringRef::npos) {
-  if (((!S.getLangOpts().OpenCLCPlusPlus &&
-(S.getLangOpts().OpenCLVersion < 200)) ||
-   (S.getLangOpts().OpenCLVersion == 300 &&
-!S.getOpenCLOptions().isSupported("__opencl_c_read_write_images",
-  S.getLangOpts( ||
+  bool ReadWriteImagesUnsupportedForOCLC =
+  (S.getLangOpts().OpenCLVersion < 200) ||
+  (S.getLangOpts().OpenCLVersion == 300 &&
+   !S.getOpenCLOptions().isSupported("__opencl_c_read_write_images",
+ S.getLangOpts()));
+  if ((!S.getLangOpts().OpenCLCPlusPlus &&
+   ReadWriteImagesUnsupportedForOCLC) ||
   DeclTy->isPipeType()) {
 S.Diag(AL.getLoc(), diag::err_opencl_invalid_read_write)
 << AL << PDecl->getType() << DeclTy->isImageType();
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D105898: [OpenMP] Rework OpenMP remarks

[Johannes Doerfert via Phabricator via cfe-commits]

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

typo in commit message.

Some minor suggestions. LG




Comment at: llvm/lib/Transforms/IPO/OpenMPOpt.cpp:1211
   auto Remark = [&](OptimizationRemark OR) {
-return OR << "Parallel region in "
-  << ore::NV("OpenMPParallelDelete", 
CI->getCaller()->getName())
-  << " deleted";
+return OR << "Removing unused parallel region.";
   };

"unsused"? Maybe "side-effect free" or "empty"?



Comment at: llvm/lib/Transforms/IPO/OpenMPOpt.cpp:2811
+
+// Skip diagnostics on calls to known OpenMP runtime functinos for now.
+if (auto *CB = dyn_cast(NonCompatibleI))





Comment at: llvm/lib/Transforms/IPO/OpenMPOpt.cpp:2951
+ << "Call may contain unknown parallel regions. Use "
+ << "`__attribute__((assume(\"omp_no_openmp\")))` to 
override.";
 };




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105898

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

[clang] 10e0cdf - [PowerPC][NFC] Power ISA features for Semachecking

[Victor Huang via cfe-commits]

Author: Victor Huang
Date: 2021-07-13T10:51:25-05:00
New Revision: 10e0cdfc6526578c8892d895c0448e77cb9ba876

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

LOG: [PowerPC][NFC] Power ISA features for Semachecking

[NFC] This patch adds features for pwr7, pwr8, and pwr9 that can be
used for semachecking builtin functions that are only valid for certain
versions of ppc.

Reviewed By: nemanjai, #powerpc
Authored By: Quinn Pham 

Differential revision: https://reviews.llvm.org/D105501

Added: 


Modified: 
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/lib/Basic/Targets/PPC.cpp
clang/lib/Basic/Targets/PPC.h
clang/lib/Sema/SemaChecking.cpp
llvm/lib/Target/PowerPC/PPC.td
llvm/lib/Target/PowerPC/PPCInstrInfo.td
llvm/lib/Target/PowerPC/PPCSubtarget.cpp
llvm/lib/Target/PowerPC/PPCSubtarget.h

Removed: 




diff  --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td 
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 2d62163e3dcc0..422507cd2842b 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -9757,8 +9757,8 @@ def err_mips_builtin_requires_dspr2 : Error<
   "this builtin requires 'dsp r2' ASE, please use -mdspr2">;
 def err_mips_builtin_requires_msa : Error<
   "this builtin requires 'msa' ASE, please use -mmsa">;
-def err_ppc_builtin_only_on_pwr7 : Error<
-  "this builtin is only valid on POWER7 or later CPUs">;
+def err_ppc_builtin_only_on_arch : Error<
+  "this builtin is only valid on POWER%0 or later CPUs">;
 def err_ppc_invalid_use_mma_type : Error<
   "invalid use of PPC MMA type">;
 def err_x86_builtin_invalid_rounding : Error<

diff  --git a/clang/lib/Basic/Targets/PPC.cpp b/clang/lib/Basic/Targets/PPC.cpp
index b77b4a38bc46f..514f1a031ae79 100644
--- a/clang/lib/Basic/Targets/PPC.cpp
+++ b/clang/lib/Basic/Targets/PPC.cpp
@@ -73,6 +73,12 @@ bool 
PPCTargetInfo::handleTargetFeatures(std::vector &Features,
   HasROPProtect = true;
 } else if (Feature == "+privileged") {
   HasPrivileged = true;
+} else if (Feature == "+isa-v207-instructions") {
+  IsISA2_07 = true;
+} else if (Feature == "+isa-v30-instructions") {
+  IsISA3_0 = true;
+} else if (Feature == "+isa-v31-instructions") {
+  IsISA3_1 = true;
 }
 // TODO: Finish this list and add an assert that we've handled them
 // all.
@@ -390,6 +396,15 @@ bool PPCTargetInfo::initFeatureMap(
 .Case("e500", true)
 .Default(false);
 
+  Features["isa-v207-instructions"] = llvm::StringSwitch(CPU)
+  .Case("ppc64le", true)
+  .Case("pwr9", true)
+  .Case("pwr8", true)
+  .Default(false);
+
+  Features["isa-v30-instructions"] =
+  llvm::StringSwitch(CPU).Case("pwr9", true).Default(false);
+
   // Power10 includes all the same features as Power9 plus any features 
specific
   // to the Power10 core.
   if (CPU == "pwr10" || CPU == "power10") {
@@ -446,6 +461,7 @@ void PPCTargetInfo::addP10SpecificFeatures(
   Features["power10-vector"] = true;
   Features["pcrelative-memops"] = true;
   Features["prefix-instrs"] = true;
+  Features["isa-v31-instructions"] = true;
   return;
 }
 
@@ -476,6 +492,9 @@ bool PPCTargetInfo::hasFeature(StringRef Feature) const {
   .Case("mma", HasMMA)
   .Case("rop-protect", HasROPProtect)
   .Case("privileged", HasPrivileged)
+  .Case("isa-v207-instructions", IsISA2_07)
+  .Case("isa-v30-instructions", IsISA3_0)
+  .Case("isa-v31-instructions", IsISA3_1)
   .Default(false);
 }
 

diff  --git a/clang/lib/Basic/Targets/PPC.h b/clang/lib/Basic/Targets/PPC.h
index bd79c68ce3f71..7c14a4eb9410c 100644
--- a/clang/lib/Basic/Targets/PPC.h
+++ b/clang/lib/Basic/Targets/PPC.h
@@ -74,6 +74,9 @@ class LLVM_LIBRARY_VISIBILITY PPCTargetInfo : public 
TargetInfo {
   bool HasP10Vector = false;
   bool HasPCRelativeMemops = false;
   bool HasPrefixInstrs = false;
+  bool IsISA2_07 = false;
+  bool IsISA3_0 = false;
+  bool IsISA3_1 = false;
 
 protected:
   std::string ABI;

diff  --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index 99621a226dea6..062c7eb4a12e5 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -3275,10 +3275,18 @@ static bool isPPC_64Builtin(unsigned BuiltinID) {
 }
 
 static bool SemaFeatureCheck(Sema &S, CallExpr *TheCall,
- StringRef FeatureToCheck, unsigned DiagID) {
-  if (!S.Context.getTargetInfo().hasFeature(FeatureToCheck))
-return S.Diag(TheCall->getBeginLoc(), DiagID) << TheCall->getSourceRange();
-  return f

[PATCH] D105501: [PowerPC] Power ISA features for Semachecking

[Victor Huang 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 rG10e0cdfc6526: [PowerPC][NFC] Power ISA features for 
Semachecking (authored by NeHuang).

Changed prior to commit:
  https://reviews.llvm.org/D105501?vs=358049&id=358285#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105501

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Basic/Targets/PPC.cpp
  clang/lib/Basic/Targets/PPC.h
  clang/lib/Sema/SemaChecking.cpp
  llvm/lib/Target/PowerPC/PPC.td
  llvm/lib/Target/PowerPC/PPCInstrInfo.td
  llvm/lib/Target/PowerPC/PPCSubtarget.cpp
  llvm/lib/Target/PowerPC/PPCSubtarget.h

Index: llvm/lib/Target/PowerPC/PPCSubtarget.h
===
--- llvm/lib/Target/PowerPC/PPCSubtarget.h
+++ llvm/lib/Target/PowerPC/PPCSubtarget.h
@@ -146,6 +146,7 @@
   bool HasStoreFusion;
   bool HasAddiLoadFusion;
   bool HasAddisLoadFusion;
+  bool IsISA2_07;
   bool IsISA3_0;
   bool IsISA3_1;
   bool UseLongCalls;
@@ -319,6 +320,7 @@
 
   bool hasHTM() const { return HasHTM; }
   bool hasFloat128() const { return HasFloat128; }
+  bool isISA2_07() const { return IsISA2_07; }
   bool isISA3_0() const { return IsISA3_0; }
   bool isISA3_1() const { return IsISA3_1; }
   bool useLongCalls() const { return UseLongCalls; }
Index: llvm/lib/Target/PowerPC/PPCSubtarget.cpp
===
--- llvm/lib/Target/PowerPC/PPCSubtarget.cpp
+++ llvm/lib/Target/PowerPC/PPCSubtarget.cpp
@@ -126,6 +126,7 @@
   HasStoreFusion = false;
   HasAddiLoadFusion = false;
   HasAddisLoadFusion = false;
+  IsISA2_07 = false;
   IsISA3_0 = false;
   IsISA3_1 = false;
   UseLongCalls = false;
Index: llvm/lib/Target/PowerPC/PPCInstrInfo.td
===
--- llvm/lib/Target/PowerPC/PPCInstrInfo.td
+++ llvm/lib/Target/PowerPC/PPCInstrInfo.td
@@ -1176,6 +1176,7 @@
 : Predicate<"!Subtarget->getTargetMachine().Options.NoNaNsFPMath">;
 def HasBPERMD : Predicate<"Subtarget->hasBPERMD()">;
 def HasExtDiv : Predicate<"Subtarget->hasExtDiv()">;
+def IsISA2_07 : Predicate<"Subtarget->isISA2_07()">;
 def IsISA3_0 : Predicate<"Subtarget->isISA3_0()">;
 def HasFPU : Predicate<"Subtarget->hasFPU()">;
 def PCRelativeMemops : Predicate<"Subtarget->hasPCRelativeMemops()">;
Index: llvm/lib/Target/PowerPC/PPC.td
===
--- llvm/lib/Target/PowerPC/PPC.td
+++ llvm/lib/Target/PowerPC/PPC.td
@@ -210,9 +210,13 @@
 def DeprecatedDST: SubtargetFeature<"", "DeprecatedDST", "true",
   "Treat vector data stream cache control instructions as deprecated">;
 
+def FeatureISA2_07 : SubtargetFeature<"isa-v207-instructions", "IsISA2_07",
+  "true",
+  "Enable instructions in ISA 2.07.">;
 def FeatureISA3_0 : SubtargetFeature<"isa-v30-instructions", "IsISA3_0",
  "true",
- "Enable instructions in ISA 3.0.">;
+ "Enable instructions in ISA 3.0.",
+ [FeatureISA2_07]>;
 def FeatureISA3_1 : SubtargetFeature<"isa-v31-instructions", "IsISA3_1",
  "true",
  "Enable instructions in ISA 3.1.",
Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -3275,10 +3275,18 @@
 }
 
 static bool SemaFeatureCheck(Sema &S, CallExpr *TheCall,
- StringRef FeatureToCheck, unsigned DiagID) {
-  if (!S.Context.getTargetInfo().hasFeature(FeatureToCheck))
-return S.Diag(TheCall->getBeginLoc(), DiagID) << TheCall->getSourceRange();
-  return false;
+ StringRef FeatureToCheck, unsigned DiagID,
+ StringRef DiagArg = "") {
+  if (S.Context.getTargetInfo().hasFeature(FeatureToCheck))
+return false;
+
+  if (DiagArg.empty())
+S.Diag(TheCall->getBeginLoc(), DiagID) << TheCall->getSourceRange();
+  else
+S.Diag(TheCall->getBeginLoc(), DiagID)
+<< DiagArg << TheCall->getSourceRange();
+
+  return true;
 }
 
 bool Sema::CheckPPCBuiltinFunctionCall(const TargetInfo &TI, unsigned BuiltinID,
@@ -3320,17 +3328,17 @@
   case PPC::BI__builtin_divde:
   case PPC::BI__builtin_divdeu:
 return SemaFeatureCheck(*this, TheCall, "extdiv",
-diag::err_ppc_builtin_only_on_pwr7);
+diag::err_ppc_builtin_only_on_arch, "7");
   case PPC::BI__builtin_bpermd:
 return SemaFeatureCheck(*this, TheCall, "bpermd",
-diag::err_ppc_builtin_o

[PATCH] D105637: [clang][Analyzer] Add symbol uninterestingness to bug report.

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

balazske added inline comments.



Comment at: clang/lib/StaticAnalyzer/Core/BugReporter.cpp:2260-2261
+  InterestingSymbols.erase(sym);
+  if (const auto *meta = dyn_cast(sym))
+markNotInteresting(meta->getRegion());
+}

NoQ wrote:
> You're saying, e.g., "If string length is not interesting then neither is the 
> string itself". Or, dunno, "If the raw pointer value is not interesting then 
> neither is a smart pointer that was holding it".
> 
> I'm not sure about that. I'm pretty sure that no checkers are currently 
> affected by this code but I still don't understand your point.
> 
> I don't understand the original code in `markInteresting()` either; do we 
> have even one test to cover it?
> 
> Also note that what you've written is not a correct negation of the original 
> code. The correct negation (that would keep the region-metadata relationship 
> in sync as originally intended) would be "if the region loses 
> interestingness, so does the metadata". Or it has to go both ways. I'm still 
> not sure if any of this matters though.
> 
> Maybe we should eliminate these extra clauses entirely. If you're not willing 
> to investigate whether this is all dead code or it actually does something, 
> I'd like to suggest a "FIXME: Is this necessary?" (or something like that) 
> both here and in the original code.
The metadata interestingness was added in rG735724fb1e78 long ago, there is not 
more information. All test pass if the code is removed (but it may have impacts 
on bug paths). Is it not the job of the checker to mark the interestingness of 
objects? I assume that the meaning of `meta->getRegion()` is checker dependent 
(the string or smart pointer or something totally different?) and the checker 
should know what to make interesting (or not). The `markInteresting()` has no 
test (what I could extend here), it may be implicitly tested by the other 
checker tests.



Comment at: clang/lib/StaticAnalyzer/Core/BugReporter.cpp:2260-2261
+  InterestingSymbols.erase(sym);
+  if (const auto *meta = dyn_cast(sym))
+markNotInteresting(meta->getRegion());
+}

balazske wrote:
> NoQ wrote:
> > You're saying, e.g., "If string length is not interesting then neither is 
> > the string itself". Or, dunno, "If the raw pointer value is not interesting 
> > then neither is a smart pointer that was holding it".
> > 
> > I'm not sure about that. I'm pretty sure that no checkers are currently 
> > affected by this code but I still don't understand your point.
> > 
> > I don't understand the original code in `markInteresting()` either; do we 
> > have even one test to cover it?
> > 
> > Also note that what you've written is not a correct negation of the 
> > original code. The correct negation (that would keep the region-metadata 
> > relationship in sync as originally intended) would be "if the region loses 
> > interestingness, so does the metadata". Or it has to go both ways. I'm 
> > still not sure if any of this matters though.
> > 
> > Maybe we should eliminate these extra clauses entirely. If you're not 
> > willing to investigate whether this is all dead code or it actually does 
> > something, I'd like to suggest a "FIXME: Is this necessary?" (or something 
> > like that) both here and in the original code.
> The metadata interestingness was added in rG735724fb1e78 long ago, there is 
> not more information. All test pass if the code is removed (but it may have 
> impacts on bug paths). Is it not the job of the checker to mark the 
> interestingness of objects? I assume that the meaning of `meta->getRegion()` 
> is checker dependent (the string or smart pointer or something totally 
> different?) and the checker should know what to make interesting (or not). 
> The `markInteresting()` has no test (what I could extend here), it may be 
> implicitly tested by the other checker tests.
The mark of associated region as interesting may not work correct if there are 
multiple metadata symbols for the same region, which should be a possible case. 
Specially the remove of interestingness is not correct in this way. So I would 
remove the `if (const auto *meta = dyn_cast(sym))` part.
A better approach can be to make the interestingness depend totally on the 
associated region for metadata symbol. If a symbol is metadata, its 
interestingness is that of the region. And only the region is inserted into the 
map. `isInteresting` can be updated for this purpose.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105637

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

[PATCH] D105898: [OpenMP] Rework OpenMP remarks

[Joseph Huber via Phabricator via cfe-commits]

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

Addressing comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105898

Files:
  clang/test/OpenMP/remarks_parallel_in_multiple_target_state_machines.c
  clang/test/OpenMP/remarks_parallel_in_target_state_machine.c
  llvm/lib/Transforms/IPO/AttributorAttributes.cpp
  llvm/lib/Transforms/IPO/OpenMPOpt.cpp
  llvm/test/Transforms/OpenMP/custom_state_machines_remarks.ll
  llvm/test/Transforms/OpenMP/deduplication_remarks.ll
  llvm/test/Transforms/OpenMP/globalization_remarks.ll
  llvm/test/Transforms/OpenMP/parallel_deletion_remarks.ll
  llvm/test/Transforms/OpenMP/remove_globalization.ll
  llvm/test/Transforms/OpenMP/spmdization_remarks.ll

Index: llvm/test/Transforms/OpenMP/spmdization_remarks.ll
===
--- llvm/test/Transforms/OpenMP/spmdization_remarks.ll
+++ llvm/test/Transforms/OpenMP/spmdization_remarks.ll
@@ -1,12 +1,13 @@
 ; RUN: opt -passes=openmp-opt -pass-remarks=openmp-opt -pass-remarks-missed=openmp-opt -pass-remarks-analysis=openmp-opt -disable-output < %s 2>&1 | FileCheck %s
 target triple = "nvptx64"
 
-; CHECK: remark: llvm/test/Transforms/OpenMP/spmdization_remarks.c:13:5: Kernel will be executed in generic-mode due to this potential side-effect, consider to add `__attribute__((assume("ompx_spmd_amenable")))` to the called function 'unknown'.
-; CHECK: remark: llvm/test/Transforms/OpenMP/spmdization_remarks.c:15:5: Kernel will be executed in generic-mode due to this potential side-effect, consider to add `__attribute__((assume("ompx_spmd_amenable")))` to the called function 'unknown'.
-; CHECK: remark: llvm/test/Transforms/OpenMP/spmdization_remarks.c:11:1: Generic-mode kernel is executed with a customized state machine that requires a fallback [1 known parallel regions, 2 unkown parallel regions] (bad).
-; CHECK: remark: llvm/test/Transforms/OpenMP/spmdization_remarks.c:13:5: State machine fallback caused by this call. If it is a false positive, use `__attribute__((assume("omp_no_openmp")))` (or "omp_no_parallelism").
-; CHECK: remark: llvm/test/Transforms/OpenMP/spmdization_remarks.c:15:5: State machine fallback caused by this call. If it is a false positive, use `__attribute__((assume("omp_no_openmp")))` (or "omp_no_parallelism").
-; CHECK: remark: llvm/test/Transforms/OpenMP/spmdization_remarks.c:20:1: Generic-mode kernel is changed to SPMD-mode.
+; CHECK: remark: llvm/test/Transforms/OpenMP/spmdization_remarks.c:13:5: Value has potential side effects preventing SPMD-mode execution. Add `__attribute__((assume("ompx_spmd_amenable")))` to the called function to override.
+; CHECK: remark: llvm/test/Transforms/OpenMP/spmdization_remarks.c:15:5: Value has potential side effects preventing SPMD-mode execution. Add `__attribute__((assume("ompx_spmd_amenable")))` to the called function to override.
+; CHECK: remark: llvm/test/Transforms/OpenMP/spmdization_remarks.c:11:1: Generic-mode kernel is executed with a customized state machine that requires a fallback [1 known parallel regions, 2 unkown parallel regions].
+; CHECK: remark: llvm/test/Transforms/OpenMP/spmdization_remarks.c:13:5: Call may contain unknown parallel regions. Use `__attribute__((assume("omp_no_openmp")))` to override.
+; CHECK: remark: llvm/test/Transforms/OpenMP/spmdization_remarks.c:15:5: Call may contain unknown parallel regions. Use `__attribute__((assume("omp_no_openmp")))` to override.
+; CHECK: remark: llvm/test/Transforms/OpenMP/spmdization_remarks.c:20:1: Transformed Generic-mode kernel to SPMD-mode.
+
 
 ;; void unknown(void);
 ;; void known(void) {
Index: llvm/test/Transforms/OpenMP/remove_globalization.ll
===
--- llvm/test/Transforms/OpenMP/remove_globalization.ll
+++ llvm/test/Transforms/OpenMP/remove_globalization.ll
@@ -4,7 +4,7 @@
 target datalayout = "e-i64:64-i128:128-v16:16-v32:32-n16:32:64"
 target triple = "nvptx64"
 
-; CHECK-REMARKS: remark: remove_globalization.c:4:2: Could not move globalized variable to the stack as variable is potentially captured in call; mark parameter as `__attribute__((noescape))` to override.
+; CHECK-REMARKS: remark: remove_globalization.c:4:2: Could not move globalized variable to the stack. Variable is potentially captured in call. Mark parameter as `__attribute__((noescape))` to override.
 ; CHECK-REMARKS: remark: remove_globalization.c:2:2: Moving globalized variable to the stack.
 ; CHECK-REMARKS: remark: remove_globalization.c:6:2: Moving globalized variable to the stack.
 ; CHECK-REMARKS: remark: remove_globalization.c:4:2: Found thread data sharing on the GPU. Expect degraded performance due to data globalization.
Index: llvm/test/Transforms/OpenMP/parallel_deletion_remarks.ll
===
--- llvm/test/Transforms/OpenMP/parallel_del

[clang] f1aca5a - [PowerPC] Fix L[D|W]ARX Implementation

[Albion Fung via cfe-commits]

Author: Albion Fung
Date: 2021-07-13T11:02:07-05:00
New Revision: f1aca5ac96ebd0beadfa68a474c5947d3bc8c109

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

LOG: [PowerPC] Fix L[D|W]ARX Implementation

LDARX and LWARX sometimes gets optimized out by the compiler
when it is critical to the correctness of the code. This inline asm generation
ensures that it preserved.

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

Added: 


Modified: 
clang/lib/CodeGen/CGBuiltin.cpp
clang/test/CodeGen/builtins-ppc-xlcompat-LoadReseve-StoreCond-64bit-only.c
clang/test/CodeGen/builtins-ppc-xlcompat-LoadReseve-StoreCond.c
llvm/include/llvm/IR/IntrinsicsPowerPC.td
llvm/lib/Target/PowerPC/PPCInstr64Bit.td
llvm/lib/Target/PowerPC/PPCInstrInfo.td

llvm/test/CodeGen/PowerPC/builtins-ppc-xlcompat-LoadReserve-StoreCond-64bit-only.ll
llvm/test/CodeGen/PowerPC/builtins-ppc-xlcompat-LoadReserve-StoreCond.ll

Removed: 




diff  --git a/clang/lib/CodeGen/CGBuiltin.cpp b/clang/lib/CodeGen/CGBuiltin.cpp
index 355ed8ffbfea1..baa1436954183 100644
--- a/clang/lib/CodeGen/CGBuiltin.cpp
+++ b/clang/lib/CodeGen/CGBuiltin.cpp
@@ -994,6 +994,46 @@ static llvm::Value *EmitBitTestIntrinsic(CodeGenFunction 
&CGF,
   ShiftedByte, llvm::ConstantInt::get(CGF.Int8Ty, 1), "bittest.res");
 }
 
+static llvm::Value *emitPPCLoadReserveIntrinsic(CodeGenFunction &CGF,
+unsigned BuiltinID,
+const CallExpr *E) {
+  Value *Addr = CGF.EmitScalarExpr(E->getArg(0));
+
+  SmallString<64> Asm;
+  raw_svector_ostream AsmOS(Asm);
+  llvm::IntegerType *RetType = CGF.Int32Ty;
+
+  switch (BuiltinID) {
+  case clang::PPC::BI__builtin_ppc_ldarx:
+AsmOS << "ldarx ";
+RetType = CGF.Int64Ty;
+break;
+  case clang::PPC::BI__builtin_ppc_lwarx:
+AsmOS << "lwarx ";
+RetType = CGF.Int32Ty;
+break;
+  default:
+llvm_unreachable("Expected only PowerPC load reserve intrinsics");
+  }
+
+  AsmOS << "$0, ${1:y}";
+
+  std::string Constraints = "=r,*Z,~{memory}";
+  std::string MachineClobbers = CGF.getTarget().getClobbers();
+  if (!MachineClobbers.empty()) {
+Constraints += ',';
+Constraints += MachineClobbers;
+  }
+
+  llvm::Type *IntPtrType = RetType->getPointerTo();
+  llvm::FunctionType *FTy =
+  llvm::FunctionType::get(RetType, {IntPtrType}, false);
+
+  llvm::InlineAsm *IA =
+  llvm::InlineAsm::get(FTy, Asm, Constraints, /*hasSideEffects=*/true);
+  return CGF.Builder.CreateCall(IA, {Addr});
+}
+
 namespace {
 enum class MSVCSetJmpKind {
   _setjmpex,
@@ -15532,6 +15572,9 @@ Value *CodeGenFunction::EmitPPCBuiltinExpr(unsigned 
BuiltinID,
 return MakeBinaryAtomicValue(*this, AtomicRMWInst::Xchg, E,
  llvm::AtomicOrdering::Monotonic);
   }
+  case PPC::BI__builtin_ppc_ldarx:
+  case PPC::BI__builtin_ppc_lwarx:
+return emitPPCLoadReserveIntrinsic(*this, BuiltinID, E);
   }
 }
 

diff  --git 
a/clang/test/CodeGen/builtins-ppc-xlcompat-LoadReseve-StoreCond-64bit-only.c 
b/clang/test/CodeGen/builtins-ppc-xlcompat-LoadReseve-StoreCond-64bit-only.c
index 80bb4de424a13..f0a8ff184311a 100644
--- a/clang/test/CodeGen/builtins-ppc-xlcompat-LoadReseve-StoreCond-64bit-only.c
+++ b/clang/test/CodeGen/builtins-ppc-xlcompat-LoadReseve-StoreCond-64bit-only.c
@@ -1,27 +1,23 @@
 // RUN: not %clang_cc1 -triple=powerpc-unknown-aix -emit-llvm %s -o - 2>&1 |\
 // RUN: FileCheck %s --check-prefix=CHECK32-ERROR
-// RUN: %clang_cc1 -triple=powerpc64-unknown-aix -emit-llvm %s -o - | \
+// RUN: %clang_cc1 -O2 -triple=powerpc64-unknown-aix -emit-llvm %s -o - | \
 // RUN: FileCheck %s --check-prefix=CHECK64
-// RUN: %clang_cc1 -triple=powerpc64le-unknown-unknown -emit-llvm %s \
+// RUN: %clang_cc1 -O2 -triple=powerpc64le-unknown-unknown -emit-llvm %s \
 // RUN:  -o - | FileCheck %s --check-prefix=CHECK64
-// RUN: %clang_cc1 -triple=powerpc64-unknown-unknown -emit-llvm %s \
+// RUN: %clang_cc1 -O2 -triple=powerpc64-unknown-unknown -emit-llvm %s \
 // RUN:  -o - | FileCheck %s --check-prefix=CHECK64
 
 long test_ldarx(volatile long* a) {
   // CHECK64-LABEL: @test_ldarx
-  // CHECK64: %0 = load i64*, i64** %a.addr, align 8
-  // CHECK64: %1 = bitcast i64* %0 to i8*
-  // CHECK64: %2 = call i64 @llvm.ppc.ldarx(i8* %1)
+  // CHECK64: %0 = tail call i64 asm sideeffect "ldarx $0, ${1:y}", 
"=r,*Z,~{memory}"(i64* %a)
   // CHECK32-ERROR: error: this builtin is only available on 64-bit targets
   return __ldarx(a);
 }
 
 int test_stdcx(volatile long* addr, long val) {
   // CHECK64-LABEL: @test_stdcx
-  // CHECK64: %0 = load i64*, i64** %addr.addr, align 8
-  // CHECK64: %1 = bitcast i64* %0 to i8*
-  // CHECK64: %2 = load i64, i64* %val.addr, align 8
-  // CHECK64: %3 = 

[PATCH] D105858: opencl-c.h: add 3.0 optional extension support for a few more bits

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added a subscriber: azabaznov.
Anastasia added inline comments.



Comment at: clang/lib/Headers/opencl-c-base.h:329
 #endif // defined(__opencl_c_atomic_scope_all_devices)
-#if defined(cl_intel_subgroups) || defined(cl_khr_subgroups)
+#if defined(cl_intel_subgroups) || defined(cl_khr_subgroups) || 
defined(__opencl_c_subgroups)
   memory_scope_sub_group = __OPENCL_MEMORY_SCOPE_SUB_GROUP

We had a discussion with @azabaznov around features that are aliasing each 
other and we have discussed to use one feature macro for those. Clang should 
already ensure that both are set/unset simultaneously? And for those that are 
not set in clang we can set them correctly here in the header directly.




Comment at: clang/lib/Headers/opencl-c.h:15290
 uint__ovld get_num_sub_groups(void);
-#if defined(__OPENCL_CPP_VERSION__) || (__OPENCL_C_VERSION__ >= CL_VERSION_2_0)
+#if defined(__opencl_c_subgroups)
 uint__ovld get_enqueued_num_sub_groups(void);

I think that here guarding the OpenCL version is correct?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105858

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

[PATCH] D105898: [OpenMP] Rework OpenMP remarks

[Joseph Huber via Phabricator via cfe-commits]

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

Forgot to update two tests.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105898

Files:
  clang/test/OpenMP/remarks_parallel_in_multiple_target_state_machines.c
  clang/test/OpenMP/remarks_parallel_in_target_state_machine.c
  llvm/lib/Transforms/IPO/AttributorAttributes.cpp
  llvm/lib/Transforms/IPO/OpenMPOpt.cpp
  llvm/test/Transforms/OpenMP/custom_state_machines_remarks.ll
  llvm/test/Transforms/OpenMP/deduplication_remarks.ll
  llvm/test/Transforms/OpenMP/globalization_remarks.ll
  llvm/test/Transforms/OpenMP/parallel_deletion_remarks.ll
  llvm/test/Transforms/OpenMP/remove_globalization.ll
  llvm/test/Transforms/OpenMP/spmdization_remarks.ll

Index: llvm/test/Transforms/OpenMP/spmdization_remarks.ll
===
--- llvm/test/Transforms/OpenMP/spmdization_remarks.ll
+++ llvm/test/Transforms/OpenMP/spmdization_remarks.ll
@@ -1,12 +1,13 @@
 ; RUN: opt -passes=openmp-opt -pass-remarks=openmp-opt -pass-remarks-missed=openmp-opt -pass-remarks-analysis=openmp-opt -disable-output < %s 2>&1 | FileCheck %s
 target triple = "nvptx64"
 
-; CHECK: remark: llvm/test/Transforms/OpenMP/spmdization_remarks.c:13:5: Kernel will be executed in generic-mode due to this potential side-effect, consider to add `__attribute__((assume("ompx_spmd_amenable")))` to the called function 'unknown'.
-; CHECK: remark: llvm/test/Transforms/OpenMP/spmdization_remarks.c:15:5: Kernel will be executed in generic-mode due to this potential side-effect, consider to add `__attribute__((assume("ompx_spmd_amenable")))` to the called function 'unknown'.
-; CHECK: remark: llvm/test/Transforms/OpenMP/spmdization_remarks.c:11:1: Generic-mode kernel is executed with a customized state machine that requires a fallback [1 known parallel regions, 2 unkown parallel regions] (bad).
-; CHECK: remark: llvm/test/Transforms/OpenMP/spmdization_remarks.c:13:5: State machine fallback caused by this call. If it is a false positive, use `__attribute__((assume("omp_no_openmp")))` (or "omp_no_parallelism").
-; CHECK: remark: llvm/test/Transforms/OpenMP/spmdization_remarks.c:15:5: State machine fallback caused by this call. If it is a false positive, use `__attribute__((assume("omp_no_openmp")))` (or "omp_no_parallelism").
-; CHECK: remark: llvm/test/Transforms/OpenMP/spmdization_remarks.c:20:1: Generic-mode kernel is changed to SPMD-mode.
+; CHECK: remark: llvm/test/Transforms/OpenMP/spmdization_remarks.c:13:5: Value has potential side effects preventing SPMD-mode execution. Add `__attribute__((assume("ompx_spmd_amenable")))` to the called function to override.
+; CHECK: remark: llvm/test/Transforms/OpenMP/spmdization_remarks.c:15:5: Value has potential side effects preventing SPMD-mode execution. Add `__attribute__((assume("ompx_spmd_amenable")))` to the called function to override.
+; CHECK: remark: llvm/test/Transforms/OpenMP/spmdization_remarks.c:11:1: Generic-mode kernel is executed with a customized state machine that requires a fallback [1 known parallel regions, 2 unkown parallel regions].
+; CHECK: remark: llvm/test/Transforms/OpenMP/spmdization_remarks.c:13:5: Call may contain unknown parallel regions. Use `__attribute__((assume("omp_no_parallelism")))` to override.
+; CHECK: remark: llvm/test/Transforms/OpenMP/spmdization_remarks.c:15:5: Call may contain unknown parallel regions. Use `__attribute__((assume("omp_no_parallelism")))` to override.
+; CHECK: remark: llvm/test/Transforms/OpenMP/spmdization_remarks.c:20:1: Transformed Generic-mode kernel to SPMD-mode.
+
 
 ;; void unknown(void);
 ;; void known(void) {
Index: llvm/test/Transforms/OpenMP/remove_globalization.ll
===
--- llvm/test/Transforms/OpenMP/remove_globalization.ll
+++ llvm/test/Transforms/OpenMP/remove_globalization.ll
@@ -4,7 +4,7 @@
 target datalayout = "e-i64:64-i128:128-v16:16-v32:32-n16:32:64"
 target triple = "nvptx64"
 
-; CHECK-REMARKS: remark: remove_globalization.c:4:2: Could not move globalized variable to the stack as variable is potentially captured in call; mark parameter as `__attribute__((noescape))` to override.
+; CHECK-REMARKS: remark: remove_globalization.c:4:2: Could not move globalized variable to the stack. Variable is potentially captured in call. Mark parameter as `__attribute__((noescape))` to override.
 ; CHECK-REMARKS: remark: remove_globalization.c:2:2: Moving globalized variable to the stack.
 ; CHECK-REMARKS: remark: remove_globalization.c:6:2: Moving globalized variable to the stack.
 ; CHECK-REMARKS: remark: remove_globalization.c:4:2: Found thread data sharing on the GPU. Expect degraded performance due to data globalization.
Index: llvm/test/Transforms/OpenMP/parallel_deletion_remarks.ll
===
--- llvm/test/Transforms/Op

[PATCH] D105858: opencl-c.h: add 3.0 optional extension support for a few more bits

[Anton Zabaznov via Phabricator via cfe-commits]

azabaznov added inline comments.



Comment at: clang/lib/Headers/opencl-c-base.h:329
 #endif // defined(__opencl_c_atomic_scope_all_devices)
-#if defined(cl_intel_subgroups) || defined(cl_khr_subgroups)
+#if defined(cl_intel_subgroups) || defined(cl_khr_subgroups) || 
defined(__opencl_c_subgroups)
   memory_scope_sub_group = __OPENCL_MEMORY_SCOPE_SUB_GROUP

Anastasia wrote:
> We had a discussion with @azabaznov around features that are aliasing each 
> other and we have discussed to use one feature macro for those. Clang should 
> already ensure that both are set/unset simultaneously? And for those that are 
> not set in clang we can set them correctly here in the header directly.
> 
Yeah, I we did. Note that this is applicable to fp64 and 3d image writes, while 
__openc_c_subgroups and cl_khr_subgroups are not equivalent as extension 
requires subgroup-independent forward progress but subgroup-independent forward 
progress is optional in OpenCL C 3.0. I'll try submit a patch for 3d image 
writes feature macro support this week.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105858

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

[PATCH] D105328: [Frontend] Only compile modules if not already finalized

[Volodymyr Sapsai via Phabricator via cfe-commits]

vsapsai added inline comments.



Comment at: clang/lib/Frontend/CompilerInstance.cpp:1063
+<< ModuleName;
+return ImportingInstance.getFrontendOpts().AllowPCMWithCompilerErrors;
+  }

Can we get in infinite loop with `AllowPCMWithCompilerErrors = true`? 
Specifically, I'm thinking about the scenario

1. `compileModuleAndReadAST` obtains a file lock and calls `compileModule`
2. `compileModule` calls `compileModuleImpl`
3. Module is finalized but `AllowPCMWithCompilerErrors` is true, so 
`compileModuleImpl` returns true
4. `compileModule` returns true
5. `compileModuleAndReadAST` tries to read AST because compilation was 
successful
6. AST is out of date, so `compileModuleAndReadAST` decides to try again, goto 1

Haven't tried to reproduce it locally but even if this scenario is impossible, 
a corresponding test case can be useful.



Comment at: clang/lib/Serialization/ASTReader.cpp:2923
   if (!BuildDir || *BuildDir != M->Directory) {
-if ((ClientLoadCapabilities & ARR_OutOfDate) == 0)
+if (diagnoseOutOfDate(F.FileName, ClientLoadCapabilities))
   Diag(diag::err_imported_module_relocated)

I'm thinking if in case of finalized modules diagnostic messages are good 
enough. One concern is it won't be clear why a module wasn't rebuilt. It can be 
already confusing for precompiled headers and I'm afraid we won't be able to 
detect `isPCMFinal` code path without a debugger. Though I don't know how bad 
that would be in practice.

Another concern is that retrying a compilation should succeed as for a new 
process we have a new InMemoryModuleCache and `isPCMFinal` should return false. 
So we might have non-deterministic behavior and some of the valid error 
messages can seem to be non-deterministic and not reliable. I was thinking 
about adding a note in case we are dealing with `isPCMFinal` to distinguish 
these cases but not sure it is a good idea.



Comment at: clang/lib/Serialization/ASTReader.cpp:5929
 
+bool ASTReader::diagnoseOutOfDate(StringRef ModuleFileName,
+  unsigned int ClientLoadCapabilities) {

Based on the rest of the code in clang, the expectation for `diagnose...` 
methods is to emit diagnostic in some cases. Personally, I'm often confused 
what true/false means for these methods, so I'm thinking about renaming the 
method to something like isRecoverableOutOfDateModule, 
canRecoverOutOfDateModule or some such. Feel free to pick a name you believe is 
appropriate, mine are just examples.



Comment at: clang/unittests/Serialization/ModuleCacheTest.cpp:125-126
+  ASSERT_TRUE(Invocation2);
+  CompilerInstance Instance2(Instance.getPCHContainerOperations(),
+ &Instance.getModuleCache());
+  Instance2.setDiagnostics(Diags.get());

bnbarham wrote:
> vsapsai wrote:
> > Haven't rechecked the code more carefully but had an idea is that if we 
> > want to allow InMemoryModuleCache reuse between multiple CompilerInstance, 
> > safer API would be to transfer ModuleCache ownership to the new 
> > CompilerInstance and maybe make all records in the cache purgeable. But 
> > that's applicable only if ModuleCache reuse is important.
> Every module compilation runs in a separate thread with a separate 
> CompilerInstance (but shared cache). So ownership would have to be 
> transferred back to the original instance once complete. I might be 
> misunderstanding what you mean here though.
> 
> Another point to note is that eg. Swift doesn't actually create another 
> CompilerInstance, it's just re-used across module loading and 
> `CompilerInstance::loadModule` is called directly. I went with this way in 
> the test as otherwise we'd need to inline most of `ExecuteAction` and have a 
> custom `FrontendAction`.
Thanks for the explanation. I haven't considered how the API is used right now, 
so please discard my earlier comment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105328

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

[PATCH] D105254: [RISCV] Support machine constraint "S"

[Jessica Clarke via Phabricator via cfe-commits]

jrtc27 added inline comments.



Comment at: llvm/test/CodeGen/RISCV/inline-asm-S-constraint.ll:27
+
+; Function Attrs: nofree nosync nounwind readnone
+define dso_local i8* @constraint_S_label() {

Not needed


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105254

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

[PATCH] D105904: [clangd] Support `#pragma mark` in the outline

[David Goldman via Phabricator via cfe-commits]

dgoldman created this revision.
Herald added subscribers: usaxena95, kadircet, arphaman, kbarton, mgorny, 
nemanjai.
dgoldman requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.
Herald added projects: clang, clang-tools-extra.

Xcode uses `#pragma mark -` to draw a divider in the outline view
and `#pragma mark Note` to add `Note` in the outline view. For more
information, see https://nshipster.com/pragma/.

Since the LSP spec doesn't contain dividers for the symbol outline,
instead we treat `#pragma mark -` as a group with children - the
decls that come after it.

Work left to do:

- Support some edge cases where the document symbols may not be nested / sorted 
properly.

- Consider supporting `#pragma mark`s before any decls

- Consider supporting the following:

  #pragma mark -
  #pragma mark Bar

by treating it like `#pragma mark - Bar` instead of an unnamed
group followed by a named mark.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D105904

Files:
  clang-tools-extra/clangd/CMakeLists.txt
  clang-tools-extra/clangd/FindSymbols.cpp
  clang-tools-extra/clangd/ParsedAST.cpp
  clang-tools-extra/clangd/ParsedAST.h
  clang-tools-extra/clangd/Protocol.h
  clang-tools-extra/clangd/TextMarks.cpp
  clang-tools-extra/clangd/TextMarks.h
  clang-tools-extra/clangd/unittests/FindSymbolsTests.cpp
  clang/include/clang/Lex/PPCallbacks.h

Index: clang/include/clang/Lex/PPCallbacks.h
===
--- clang/include/clang/Lex/PPCallbacks.h
+++ clang/include/clang/Lex/PPCallbacks.h
@@ -492,6 +492,11 @@
 Second->PragmaComment(Loc, Kind, Str);
   }
 
+  void PragmaMark(SourceLocation Loc, StringRef Trivia) override {
+First->PragmaMark(Loc, Trivia);
+Second->PragmaMark(Loc, Trivia);
+  }
+
   void PragmaDetectMismatch(SourceLocation Loc, StringRef Name,
 StringRef Value) override {
 First->PragmaDetectMismatch(Loc, Name, Value);
Index: clang-tools-extra/clangd/unittests/FindSymbolsTests.cpp
===
--- clang-tools-extra/clangd/unittests/FindSymbolsTests.cpp
+++ clang-tools-extra/clangd/unittests/FindSymbolsTests.cpp
@@ -1027,6 +1027,106 @@
 AllOf(WithName("-pur"), WithKind(SymbolKind::Method));
 }
 
+TEST(DocumentSymbolsTest, PragmaMarkGroups) {
+  TestTU TU;
+  TU.ExtraArgs = {"-xobjective-c++", "-Wno-objc-root-class"};
+  Annotations Main(R"cpp(
+  $DogDef[[@interface Dog
+  @end]]
+
+  $DogImpl[[@implementation Dog
+
+  + (id)sharedDoggo { return 0; }
+
+  #pragma $Overrides[[mark - Overrides
+
+  - (id)init {
+return self;
+  }
+  - (void)bark {}]]
+
+  #pragma $Specifics[[mark - Dog Specifics
+
+  - (int)isAGoodBoy {
+return 1;
+  }]]
+  @]]end  // FIXME: Why doesn't this include the 'end'?
+
+  #pragma $End[[mark - End
+]]
+)cpp");
+  TU.Code = Main.code().str();
+  EXPECT_THAT(
+  getSymbols(TU.build()),
+  ElementsAre(
+  AllOf(WithName("Dog"), SymRange(Main.range("DogDef"))),
+  AllOf(WithName("Dog"), SymRange(Main.range("DogImpl")),
+Children(AllOf(WithName("+sharedDoggo"),
+   WithKind(SymbolKind::Method)),
+ AllOf(WithName("Overrides"),
+   SymRange(Main.range("Overrides")),
+   Children(AllOf(WithName("-init"),
+  WithKind(SymbolKind::Method)),
+AllOf(WithName("-bark"),
+  WithKind(SymbolKind::Method,
+ AllOf(WithName("Dog Specifics"),
+   SymRange(Main.range("Specifics")),
+   Children(AllOf(WithName("-isAGoodBoy"),
+  WithKind(SymbolKind::Method)),
+  AllOf(WithName("End"), SymRange(Main.range("End");
+}
+
+TEST(DocumentSymbolsTest, PragmaMarkGroupsNoNesting) {
+  TestTU TU;
+  TU.ExtraArgs = {"-xobjective-c++", "-Wno-objc-root-class"};
+  Annotations Main(R"cpp(
+  // FIXME: We miss the mark below, is it in the preamble instead?
+  // Unclear if it's worth supporting, likely very uncommon.
+  #pragma mark Helpers
+  void helpA(id obj) {}
+
+  #pragma mark -
+  #pragma mark Core
+
+  void coreMethod() {}
+)cpp");
+  TU.Code = Main.code().str();
+  EXPECT_THAT(
+  getSymbols(TU.build()),
+  ElementsAre(AllOf(WithName("helpA")), AllOf(WithName("(unnamed group)")),
+  AllOf(WithName("Core")), AllOf(WithName("coreMethod";
+}
+
+TEST(DocumentSymbolsTest, SymbolsAreSorted) {
+  TestTU TU;
+  TU.ExtraArgs = {"-xobjective-c++", "-Wno-objc-root-class"};
+  Annotations Main(R"cpp(
+  @interface MYObject
+  @end
+
+  voi