[PATCH] D122478: [PowerPC] Add max/min intrinsics to Clang and PPC backend

[Qiu Chaofan via Phabricator via cfe-commits]

qiucf added inline comments.



Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:9899
+def err_ppc_unsupported_argument_type : Error<
+  "unsupported argument type %0 for target %1">;
 def err_x86_builtin_invalid_rounding : Error<

Can we re-use `err_typecheck_convert_incompatible`?



Comment at: clang/lib/CodeGen/CGBuiltin.cpp:16290-16310
+  case PPC::BI__builtin_ppc_maxfe:
+  case PPC::BI__builtin_ppc_maxfl:
+  case PPC::BI__builtin_ppc_maxfs:
+  case PPC::BI__builtin_ppc_minfe:
+  case PPC::BI__builtin_ppc_minfl:
+  case PPC::BI__builtin_ppc_minfs: {
+if (BuiltinID == PPC::BI__builtin_ppc_maxfe)





Comment at: clang/lib/Sema/SemaChecking.cpp:3913
+  case PPC::BI__builtin_ppc_minfs: {
+// FIXME: remove below check once -mlong-double-128 is supported on AIX.
+if (Context.getTargetInfo().getTriple().isOSAIX() &&

I think we don't need this fixme.



Comment at: clang/test/CodeGen/PowerPC/builtins-ppc.c:65
+  // CHECK: call double (double, double, double, ...) @llvm.ppc.maxfl(double 
%0,
+  // double %1, double %2, double %3)
+  res = __builtin_ppc_maxfl(a, b, c, d);

Don't break CHECK lines.



Comment at: clang/test/Sema/builtins-ppc.c:13-17
+// RUN: %clang_cc1 -triple powerpc64le-unknown-unknown -DTEST_MAXMIN 
-fsyntax-only \
+// RUN: -verify %s
+
+// RUN: %clang_cc1 -triple powerpc64-ibm-aix-xcoff -DTEST_MAXMINFE_AIX 
-fsyntax-only \
+// RUN: -verify %s





Comment at: clang/test/Sema/builtins-ppc.c:63-83
+#ifdef TEST_MAXMIN
+void test_maxmin() {
+  long double fe;
+  double fl;
+  float fs;
+  __builtin_ppc_maxfe(fl, fl, fl, fl); // expected-error-re {{requires 
argument of {{.*}} type (passed in {{.*}})}}
+  __builtin_ppc_minfe(fl, fl, fl, fl); // expected-error-re {{requires 
argument of {{.*}} type (passed in {{.*}})}}

I don't know if it's convention in tests, but looks simpler.



Comment at: llvm/include/llvm/IR/IntrinsicsPowerPC.td:192
+  [llvm_float_ty, llvm_float_ty, llvm_float_ty, 
llvm_vararg_ty],
+  [IntrNoMem]>;
 }

Will we support `llvm_f128_ty`?



Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:10583-10587
+for (unsigned i = 4, e = Op.getNumOperands(); i < e; ++i) {
+  if (Op.getOperand(i).getValueType() != Op.getValueType())
+report_fatal_error("Intrinsic::ppc_[max|min]f[e|l|s] must have uniform 
"
+   "type arguments");
+}

We can make it even simpler.



Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:10594
+// Below selection order follows XLC behavior: start from the last but one
+// operand, move towards the first operand, end with the last operand.
+unsigned I, Cnt;

We don't need to mention old behavior.



Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:10595-10596
+// operand, move towards the first operand, end with the last operand.
+unsigned I, Cnt;
+I = Cnt = Op.getNumOperands() - 2;
+SDValue Res = Op.getOperand(I);





Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:10598-10602
+for (--I; Cnt != 0; --Cnt, I = (--I == 0 ? (Op.getNumOperands() - 1) : I)) 
{
+  Res = LowerSELECT_CC(
+  DAG.getSelectCC(dl, Res, Op.getOperand(I), Res, Op.getOperand(I), 
CC),
+  DAG);
+}

I don't think we need to manually call `LowerSELECT_CC` here. SelectionDAG 
knows `ppc_fp128` should not be custom lowered.

This also makes the case pass. Thus D122462 is not needed.



Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:11266
+case Intrinsic::ppc_maxfe:
+case Intrinsic::ppc_minfe:
 case Intrinsic::ppc_fnmsub:

Why only two `fe`?



Comment at: llvm/test/CodeGen/PowerPC/builtins-ppc-xlcompat-maxmin.ll:2
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc -mcpu=pwr9 -mtriple=powerpc64le-unknown-linux < %s | FileCheck %s
+

Can we add `pwr8` run line?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122478

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

[PATCH] D122556: [RISCV] Add definitions for Xiangshan processors.

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added inline comments.



Comment at: clang/test/CodeGen/RISCV/riscv-metadata.c:18
 
+// Test if Xiangshan processors work with -target-cpu
+// RUN: %clang_cc1 -triple riscv64 -emit-llvm -o - -target-cpu 
xiangshan-yanqihu %s | FileCheck -check-prefix=EMPTY-LP64 %s

RUN lines in this file are unnecessary. mcpu=sifive-u74 is an arbitrary choice 
and provides the needed coverage. Don't add more CPUs here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122556

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

[PATCH] D122556: [RISCV] Add definitions for Xiangshan processors.

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added inline comments.



Comment at: clang/test/Driver/riscv-cpus.c:146
+// MCPU-XS-NANHU: "-target-feature" "+m" "-target-feature" "+a" 
"-target-feature" "+f" "-target-feature" "+d"
+// MCPU-XS-NANHU: "-target-feature" "+c" "-target-feature" "+zba" 
"-target-feature" "+zbb" "-target-feature" "+zbc"
+// MCPU-XS-NANHU: "-target-feature" "+zbkb" "-target-feature" "+zbkc" 
"-target-feature" "+zbkx" "-target-feature" "+zbs"

So that any extra/missing target feature can be detected.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122556

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

[PATCH] D122478: [PowerPC] Add max/min intrinsics to Clang and PPC backend

[Ting Wang via Phabricator via cfe-commits]

tingwang updated this revision to Diff 418509.
tingwang added a comment.

Update test case to show that we need D122462 
 to fix the crash.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122478

Files:
  clang/include/clang/Basic/BuiltinsPPC.def
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Basic/Targets/PPC.cpp
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/Sema/SemaChecking.cpp
  clang/test/CodeGen/PowerPC/builtins-ppc.c
  clang/test/Sema/builtins-ppc.c
  llvm/include/llvm/IR/IntrinsicsPowerPC.td
  llvm/lib/Target/PowerPC/PPCISelLowering.cpp
  llvm/test/CodeGen/PowerPC/builtins-ppc-xlcompat-maxmin.ll
  llvm/test/CodeGen/PowerPC/maxmin-select-cc.ll

Index: llvm/test/CodeGen/PowerPC/maxmin-select-cc.ll
===
--- /dev/null
+++ llvm/test/CodeGen/PowerPC/maxmin-select-cc.ll
@@ -0,0 +1,12 @@
+; REQUIRES: asserts
+; RUN: not --crash llc -verify-machineinstrs -mcpu=pwr9 -mtriple=powerpc64-unknown-unknown \
+; RUN:   < %s 2>&1 | FileCheck %s
+
+declare ppc_fp128 @llvm.ppc.maxfe(ppc_fp128 %a, ppc_fp128 %b, ppc_fp128 %c, ...)
+define ppc_fp128 @test_maxfe(ppc_fp128 %a, ppc_fp128 %b, ppc_fp128 %c, ppc_fp128 %d) {
+entry:
+; CHECK: Do not know how to custom type legalize this operation!
+; CHECK: UNREACHABLE executed at {{.*}}
+  %0 = call ppc_fp128 (ppc_fp128, ppc_fp128, ppc_fp128, ...) @llvm.ppc.maxfe(ppc_fp128 %a, ppc_fp128 %b, ppc_fp128 %c, ppc_fp128 %d)
+  ret ppc_fp128 %0
+}
Index: llvm/test/CodeGen/PowerPC/builtins-ppc-xlcompat-maxmin.ll
===
--- /dev/null
+++ llvm/test/CodeGen/PowerPC/builtins-ppc-xlcompat-maxmin.ll
@@ -0,0 +1,54 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc -mcpu=pwr9 -mtriple=powerpc64le-unknown-linux < %s | FileCheck %s
+
+declare double @llvm.ppc.maxfl(double %a, double %b, double %c, ...)
+define double @test_maxfl(double %a, double %b, double %c, double %d) {
+; CHECK-LABEL: test_maxfl:
+; CHECK:   # %bb.0: # %entry
+; CHECK-NEXT:xsmaxcdp 0, 3, 2
+; CHECK-NEXT:xsmaxcdp 0, 0, 1
+; CHECK-NEXT:xsmaxcdp 1, 0, 4
+; CHECK-NEXT:blr
+entry:
+  %0 = call double (double, double, double, ...) @llvm.ppc.maxfl(double %a, double %b, double %c, double %d)
+  ret double %0
+}
+
+declare float @llvm.ppc.maxfs(float %a, float %b, float %c, ...)
+define float @test_maxfs(float %a, float %b, float %c, float %d) {
+; CHECK-LABEL: test_maxfs:
+; CHECK:   # %bb.0: # %entry
+; CHECK-NEXT:xsmaxcdp 0, 3, 2
+; CHECK-NEXT:xsmaxcdp 0, 0, 1
+; CHECK-NEXT:xsmaxcdp 1, 0, 4
+; CHECK-NEXT:blr
+entry:
+  %0 = call float (float, float, float, ...) @llvm.ppc.maxfs(float %a, float %b, float %c, float %d)
+  ret float %0
+}
+
+declare double @llvm.ppc.minfl(double %a, double %b, double %c, ...)
+define double @test_minfl(double %a, double %b, double %c, double %d) {
+; CHECK-LABEL: test_minfl:
+; CHECK:   # %bb.0: # %entry
+; CHECK-NEXT:xsmincdp 0, 3, 2
+; CHECK-NEXT:xsmincdp 0, 0, 1
+; CHECK-NEXT:xsmincdp 1, 0, 4
+; CHECK-NEXT:blr
+entry:
+  %0 = call double (double, double, double, ...) @llvm.ppc.minfl(double %a, double %b, double %c, double %d)
+  ret double %0
+}
+
+declare float @llvm.ppc.minfs(float %a, float %b, float %c, ...)
+define float @test_minfs(float %a, float %b, float %c, float %d) {
+; CHECK-LABEL: test_minfs:
+; CHECK:   # %bb.0: # %entry
+; CHECK-NEXT:xsmincdp 0, 3, 2
+; CHECK-NEXT:xsmincdp 0, 0, 1
+; CHECK-NEXT:xsmincdp 1, 0, 4
+; CHECK-NEXT:blr
+entry:
+  %0 = call float (float, float, float, ...) @llvm.ppc.minfs(float %a, float %b, float %c, float %d)
+  ret float %0
+}
Index: llvm/lib/Target/PowerPC/PPCISelLowering.cpp
===
--- llvm/lib/Target/PowerPC/PPCISelLowering.cpp
+++ llvm/lib/Target/PowerPC/PPCISelLowering.cpp
@@ -10574,6 +10574,34 @@
 dl, SDValue());
 return Result.first;
   }
+  case Intrinsic::ppc_maxfe:
+  case Intrinsic::ppc_maxfl:
+  case Intrinsic::ppc_maxfs:
+  case Intrinsic::ppc_minfe:
+  case Intrinsic::ppc_minfl:
+  case Intrinsic::ppc_minfs: {
+for (unsigned i = 4, e = Op.getNumOperands(); i < e; ++i) {
+  if (Op.getOperand(i).getValueType() != Op.getValueType())
+report_fatal_error("Intrinsic::ppc_[max|min]f[e|l|s] must have uniform "
+   "type arguments");
+}
+ISD::CondCode CC = ISD::SETGT;
+if (IntrinsicID == Intrinsic::ppc_minfe ||
+IntrinsicID == Intrinsic::ppc_minfl ||
+IntrinsicID == Intrinsic::ppc_minfs)
+  CC = ISD::SETLT;
+// Below selection order follows XLC behavior: start from the last but one
+// operand, move towards the first operand, end with the last operand.
+unsigned I, Cnt;
+I = Cnt = Op.getNumOperands() -

[PATCH] D122542: [flang][driver] Make --version and -version consistent with clang

[Andrzej Warzynski via Phabricator via cfe-commits]

awarzynski accepted this revision.
awarzynski added a comment.

Makes sense, many thanks for doing this!




Comment at: clang/include/clang/Driver/Options.td:5756
+
+}
+

[nit] IMHO this file lacks comments, hence the suggestion :) 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122542

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

[PATCH] D121984: [RISCV][NFC] Moving RVV intrinsic type related util to llvm/Support

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

Seems that there is a circular dependency problem: 
7c7e7770b7176f452d40f4e7ac545aaee1b19c4d 
 
(reverted).

I'd suggest that we revert this patch.

Having target specific stuff in LLVMSupport is always fishy. 
Yes, we have ARMAttributeParser.h, ARMBuildAttributes.h, then we have more 
RISCV* headers.
I can kinda understand it because clang-tblgen only depends on LLVMSupport.
But this circular dependency requires more thoughts, I think.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121984

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

[clang] c0eb9b4 - Revert D121984 "[RISCV][NFC] Moving RVV intrinsic type related util to llvm/Support"

[Fangrui Song via cfe-commits]

Author: Fangrui Song
Date: 2022-03-28T01:17:37-07:00
New Revision: c0eb9b4cdef6049ebabb4018d3c9dcb0dc699868

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

LOG: Revert D121984 "[RISCV][NFC] Moving RVV intrinsic type related util to 
llvm/Support"

This reverts commit ad57e10dbca2fdeff1448afc0aa1cf23d6df8736 and 
1967fd8d5e7e40a987d8f65d163c7eb8f4b9e76f

llvm/lib/Support/RISCVVIntrinsicUtils.cpp introduced llvm/TableGen includes,
a circular dependency 
https://llvm.org/docs/CodingStandards.html#library-layering
I think this particular instance is serious and should be reverted.

Added: 


Modified: 
clang/utils/TableGen/RISCVVEmitter.cpp
llvm/lib/Support/CMakeLists.txt

Removed: 
llvm/include/llvm/Support/RISCVVIntrinsicUtils.h
llvm/lib/Support/RISCVVIntrinsicUtils.cpp



diff  --git a/clang/utils/TableGen/RISCVVEmitter.cpp 
b/clang/utils/TableGen/RISCVVEmitter.cpp
index accced0292f0e..f26b1189c1e97 100644
--- a/clang/utils/TableGen/RISCVVEmitter.cpp
+++ b/clang/utils/TableGen/RISCVVEmitter.cpp
@@ -20,15 +20,211 @@
 #include "llvm/ADT/StringMap.h"
 #include "llvm/ADT/StringSet.h"
 #include "llvm/ADT/Twine.h"
-#include "llvm/Support/RISCVVIntrinsicUtils.h"
 #include "llvm/TableGen/Error.h"
 #include "llvm/TableGen/Record.h"
 #include 
 
 using namespace llvm;
-using namespace llvm::RISCV;
+using BasicType = char;
+using VScaleVal = Optional;
 
 namespace {
+
+// Exponential LMUL
+struct LMULType {
+  int Log2LMUL;
+  LMULType(int Log2LMUL);
+  // Return the C/C++ string representation of LMUL
+  std::string str() const;
+  Optional getScale(unsigned ElementBitwidth) const;
+  void MulLog2LMUL(int Log2LMUL);
+  LMULType &operator*=(uint32_t RHS);
+};
+
+// This class is compact representation of a valid and invalid RVVType.
+class RVVType {
+  enum ScalarTypeKind : uint32_t {
+Void,
+Size_t,
+Ptr
diff _t,
+UnsignedLong,
+SignedLong,
+Boolean,
+SignedInteger,
+UnsignedInteger,
+Float,
+Invalid,
+  };
+  BasicType BT;
+  ScalarTypeKind ScalarType = Invalid;
+  LMULType LMUL;
+  bool IsPointer = false;
+  // IsConstant indices are "int", but have the constant expression.
+  bool IsImmediate = false;
+  // Const qualifier for pointer to const object or object of const type.
+  bool IsConstant = false;
+  unsigned ElementBitwidth = 0;
+  VScaleVal Scale = 0;
+  bool Valid;
+
+  std::string BuiltinStr;
+  std::string ClangBuiltinStr;
+  std::string Str;
+  std::string ShortStr;
+
+public:
+  RVVType() : RVVType(BasicType(), 0, StringRef()) {}
+  RVVType(BasicType BT, int Log2LMUL, StringRef prototype);
+
+  // Return the string representation of a type, which is an encoded string for
+  // passing to the BUILTIN() macro in Builtins.def.
+  const std::string &getBuiltinStr() const { return BuiltinStr; }
+
+  // Return the clang builtin type for RVV vector type which are used in the
+  // riscv_vector.h header file.
+  const std::string &getClangBuiltinStr() const { return ClangBuiltinStr; }
+
+  // Return the C/C++ string representation of a type for use in the
+  // riscv_vector.h header file.
+  const std::string &getTypeStr() const { return Str; }
+
+  // Return the short name of a type for C/C++ name suffix.
+  const std::string &getShortStr() {
+// Not all types are used in short name, so compute the short name by
+// demanded.
+if (ShortStr.empty())
+  initShortStr();
+return ShortStr;
+  }
+
+  bool isValid() const { return Valid; }
+  bool isScalar() const { return Scale.hasValue() && Scale.getValue() == 0; }
+  bool isVector() const { return Scale.hasValue() && Scale.getValue() != 0; }
+  bool isVector(unsigned Width) const {
+return isVector() && ElementBitwidth == Width;
+  }
+  bool isFloat() const { return ScalarType == ScalarTypeKind::Float; }
+  bool isSignedInteger() const {
+return ScalarType == ScalarTypeKind::SignedInteger;
+  }
+  bool isFloatVector(unsigned Width) const {
+return isVector() && isFloat() && ElementBitwidth == Width;
+  }
+  bool isFloat(unsigned Width) const {
+return isFloat() && ElementBitwidth == Width;
+  }
+
+private:
+  // Verify RVV vector type and set Valid.
+  bool verifyType() const;
+
+  // Creates a type based on basic types of TypeRange
+  void applyBasicType();
+
+  // Applies a prototype modifier to the current type. The result maybe an
+  // invalid type.
+  void applyModifier(StringRef prototype);
+
+  // Compute and record a string for legal type.
+  void initBuiltinStr();
+  // Compute and record a builtin RVV vector type string.
+  void initClangBuiltinStr();
+  // Compute and record a type string for used in the header.
+  void initTypeStr();
+  // Compute and record a short name of a type for C/C++ name suffix.
+  void initShortStr();
+};
+
+usi

[PATCH] D121984: [RISCV][NFC] Moving RVV intrinsic type related util to llvm/Support

[Kito Cheng via Phabricator via cfe-commits]

kito-cheng added a subscriber: akuegel.
kito-cheng added a comment.

Hi @MaskRay, @akuegel has removed the `llvm/TableGen/*`, so I guess the issue 
you mentioned in 
https://github.com/llvm/llvm-project/commit/c0eb9b4cdef6049ebabb4018d3c9dcb0dc699868
 is resolved by 
https://github.com/llvm/llvm-project/commit/268f24d2ea6ae92853c68c4614722d9e769a9408

Are you happy if I recommit that with @akuegel's fix again?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121984

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

[PATCH] D121984: [RISCV][NFC] Moving RVV intrinsic type related util to llvm/Support

[Adrian Kuegel via Phabricator via cfe-commits]

akuegel added a comment.

Yes, I believe the issue with the circular dependency was already fixed by my 
latest commit. Sorry for first committing the wrong fix (just removing the 
headers without checking that they were needed to pull in raw_ostream header 
transitively).
So if you apply my fix, you should be able to land this again (or just revert 
the revert).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121984

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

[PATCH] D121984: [RISCV][NFC] Moving RVV intrinsic type related util to llvm/Support

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

Ah, sorry, I did not see that the end state of @akuegel's 3 commits fixed the 
circular dependency.

However, I think my other concern still applies: the layering seems weird that 
the RISCV stuff is placed in LLVMSupport.
Any chance it can be fixed?
You may consider a more appropriate library and possibly let clang-tblgen 
depend on it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121984

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

[PATCH] D121984: [RISCV][NFC] Moving RVV intrinsic type related util to llvm/Support

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

The original patch added 600 lines while the subject just said "Move". This may 
be fishy as well.

Having TableGen related `void RVVIntrinsic::emitCodeGenSwitchBody(raw_ostream 
&OS) const {` in LLVMSupport is also weird.

So I think this patch deserves more discussion and should not be simply 
reapplied.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121984

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

[PATCH] D122234: [clang] Link libbitint for large division of _BitInt; increase max _BitInt size

[Matthias Gehre via Phabricator via cfe-commits]

mgehre-amd marked 2 inline comments as done.
mgehre-amd added inline comments.
Herald added a subscriber: MaskRay.



Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:8422
 
+def err_int_to_float_bit_int_max_size : Error<
+  "cannot convert '_BitInt' operands of more than %0 bits to floating point">;

aaron.ballman wrote:
> mgehre-amd wrote:
> > aaron.ballman wrote:
> > > mgehre-amd wrote:
> > > > erichkeane wrote:
> > > > > Can you explain the issue here?  This is supposed to be well-defined 
> > > > > behavior.
> > > > I saw `Assertion LC != RTLIB::UNKNOWN_LIBCALL && "Don't know how to 
> > > > expand this SINT_TO_FP!"' failed.` in the backend. I think we would 
> > > > need to add library calls for floating to bitint (and vice versa) to 
> > > > the bitint library to enable that.
> > > Good catch! I think we'll need to solve that before we can enable wide 
> > > bit-width support (users are going to expect assignment and 
> > > initialization to not crash as those are basic builtin operators). FWIW, 
> > > this is a reproducer from Clang 13 where we still allowed large widths: 
> > > https://godbolt.org/z/hvzWq1KTK
> > I don't think I agree. 
> > 
> > Right now users will get a diagnostic that _BitInt > 128 bit cannot be 
> > used. With this PR, they can use them but get a diagnostic when they try to 
> > convert large _BitInts to floats or back.
> > I think there is huge value in allowing large _BitInts even when float 
> > to/from conversion will cause a clang diagnostic because at least in my use 
> > case that is pretty uncommon,
> > so I propose to move forward with this PR (lifting the limit on _BitInt 
> > width) without having the compiler-rt support for float conversions.
> > Right now users will get a diagnostic that _BitInt > 128 bit cannot be 
> > used. With this PR, they can use them but get a diagnostic when they try to 
> > convert large _BitInts to floats or back.
> 
> This is certainly better than crashing, no doubt.
> 
> > I think there is huge value in allowing large _BitInts even when float 
> > to/from conversion will cause a clang diagnostic because at least in my use 
> > case that is pretty uncommon,
> 
> That's the problem -- this is a new basic datatype; we can't make assumptions 
> that our constituent uses cases are the common pattern. My experience thus 
> far with this feature has been that people are using it for all sorts of 
> reasons in ways I wouldn't have expected, like as a big int, as a regular int 
> that doesn't integer promote, as bit-fields, etc.
> 
> To me, the bar for whether we allow large bit widths than we do today is: do 
> all basic mathematical operators on any bit-width work correctly at runtime 
> without crashing or major compile-time or runtime performance issues for the 
> given target you want to change the limit for? Assignment and conversion are 
> basic mathematical operators I'd definitely expect to work; these aren't 
> weird situations -- assigning a float to an integer happens with some 
> regularity, so there's no reason to assume it won't happen when the integer 
> is a larger bit-precise integer: https://godbolt.org/z/hx5sYbjGK.
> 
> I'd *much* rather slow-roll the feature than have people's first impression 
> be that they can't trust the feature because it's too flaky. The whole point 
> to the `BITINT_MAXWIDTH` macro is to give the user a reliable way to know 
> what bit widths are supported; lying will not do us any favors.
Ok, then let me propose the following:
- Keep the default max. _BitInt size limit at 128 with this PR
- Remove the code that emits the float-to-int/int-to-float warnings for bit 
_BitInts
- Add a cc1 option -fmax-bitint-size=N to allow overwriting the max. _BitInt 
size for tests (so we can test big division even when float-to-int isn't there 
yet)

What do you think?



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122234

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

[clang] c5d83cd - [clang][ASTImporter] Fix a bug when importing CXXDefaultInitExpr.

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

Author: Balázs Kéri
Date: 2022-03-28T10:55:26+02:00
New Revision: c5d83cdca457fd024a3f76c5e2ba649462ecde67

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

LOG: [clang][ASTImporter] Fix a bug when importing CXXDefaultInitExpr.

The "in-class initializer" expression should be set in the field of a
default initialization expression before this expression node is created.
The `CXXDefaultInitExpr` objects are created after the AST is loaded and
at import not present in the "To" AST. And the in-class initializers of
the used fields can be missing too, these must be set at import.

This fixes a github issue #54061.

Reviewed By: martong

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

Added: 
clang/test/Analysis/Inputs/ctu-cxxdefaultinitexpr-import.cpp

clang/test/Analysis/Inputs/ctu-cxxdefaultinitexpr-import.cpp.externalDefMap.ast-dump.txt
clang/test/Analysis/ctu-cxxdefaultinitexpr.cpp

Modified: 
clang/lib/AST/ASTImporter.cpp
clang/unittests/AST/ASTImporterTest.cpp

Removed: 




diff  --git a/clang/lib/AST/ASTImporter.cpp b/clang/lib/AST/ASTImporter.cpp
index 9b421c7dec7c9..a5b6e3fa3a687 100644
--- a/clang/lib/AST/ASTImporter.cpp
+++ b/clang/lib/AST/ASTImporter.cpp
@@ -3646,19 +3646,23 @@ ExpectedDecl ASTNodeImporter::VisitFieldDecl(FieldDecl 
*D) {
 // initializer of a FieldDecl might not had been instantiated in the
 // "To" context.  However, the "From" context might instantiated that,
 // thus we have to merge that.
+// Note: `hasInClassInitializer()` is not the same as non-null
+// `getInClassInitializer()` value.
 if (Expr *FromInitializer = D->getInClassInitializer()) {
-  // We don't have yet the initializer set.
-  if (FoundField->hasInClassInitializer() &&
-  !FoundField->getInClassInitializer()) {
-if (ExpectedExpr ToInitializerOrErr = import(FromInitializer))
+  if (ExpectedExpr ToInitializerOrErr = import(FromInitializer)) {
+// Import of the FromInitializer may result in the setting of
+// InClassInitializer. If not, set it here.
+assert(FoundField->hasInClassInitializer() &&
+   "Field should have an in-class initializer if it has an "
+   "expression for it.");
+if (!FoundField->getInClassInitializer())
   FoundField->setInClassInitializer(*ToInitializerOrErr);
-else {
-  // We can't return error here,
-  // since we already mapped D as imported.
-  // FIXME: warning message?
-  consumeError(ToInitializerOrErr.takeError());
-  return FoundField;
-}
+  } else {
+// We can't return error here,
+// since we already mapped D as imported.
+// FIXME: warning message?
+consumeError(ToInitializerOrErr.takeError());
+return FoundField;
   }
 }
 return FoundField;
@@ -8127,8 +8131,23 @@ ExpectedStmt 
ASTNodeImporter::VisitCXXDefaultInitExpr(CXXDefaultInitExpr *E) {
   if (!UsedContextOrErr)
 return UsedContextOrErr.takeError();
 
-  return CXXDefaultInitExpr::Create(
-  Importer.getToContext(), *ToBeginLocOrErr, *ToFieldOrErr, 
*UsedContextOrErr);
+  FieldDecl *ToField = *ToFieldOrErr;
+  assert(ToField->hasInClassInitializer() &&
+ "Field should have in-class initializer if there is a default init "
+ "expression that uses it.");
+  if (!ToField->getInClassInitializer()) {
+// The in-class initializer may be not yet set in "To" AST even if the
+// field is already there. This must be set here to make construction of
+// CXXDefaultInitExpr work.
+auto ToInClassInitializerOrErr =
+import(E->getField()->getInClassInitializer());
+if (!ToInClassInitializerOrErr)
+  return ToInClassInitializerOrErr.takeError();
+ToField->setInClassInitializer(*ToInClassInitializerOrErr);
+  }
+
+  return CXXDefaultInitExpr::Create(Importer.getToContext(), *ToBeginLocOrErr,
+ToField, *UsedContextOrErr);
 }
 
 ExpectedStmt ASTNodeImporter::VisitCXXNamedCastExpr(CXXNamedCastExpr *E) {

diff  --git a/clang/test/Analysis/Inputs/ctu-cxxdefaultinitexpr-import.cpp 
b/clang/test/Analysis/Inputs/ctu-cxxdefaultinitexpr-import.cpp
new file mode 100644
index 0..e0e356ff555cf
--- /dev/null
+++ b/clang/test/Analysis/Inputs/ctu-cxxdefaultinitexpr-import.cpp
@@ -0,0 +1,26 @@
+namespace QHashPrivate {
+template  int b;
+struct Data;
+} // namespace QHashPrivate
+
+struct QDomNodePrivate {};
+template  struct QMultiHash {
+  QHashPrivate::Data *d = nullptr;
+};
+
+struct QDomNamedNodeMapPrivate {
+  QMultiHash<> map;
+};
+struct 

[PATCH] D120824: [clang][ASTImporter] Fix a bug when importing CXXDefaultInitExpr.

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

This revision was automatically updated to reflect the committed changes.
Closed by commit rGc5d83cdca457: [clang][ASTImporter] Fix a bug when importing 
CXXDefaultInitExpr. (authored by balazske).

Changed prior to commit:
  https://reviews.llvm.org/D120824?vs=414642&id=418521#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120824

Files:
  clang/lib/AST/ASTImporter.cpp
  clang/test/Analysis/Inputs/ctu-cxxdefaultinitexpr-import.cpp
  
clang/test/Analysis/Inputs/ctu-cxxdefaultinitexpr-import.cpp.externalDefMap.ast-dump.txt
  clang/test/Analysis/ctu-cxxdefaultinitexpr.cpp
  clang/unittests/AST/ASTImporterTest.cpp

Index: clang/unittests/AST/ASTImporterTest.cpp
===
--- clang/unittests/AST/ASTImporterTest.cpp
+++ clang/unittests/AST/ASTImporterTest.cpp
@@ -530,6 +530,21 @@
has(floatLiteral(equals(1.0);
 }
 
+const internal::VariadicDynCastAllOfMatcher
+cxxDefaultInitExpr;
+
+TEST_P(ImportExpr, ImportCXXDefaultInitExpr) {
+  MatchVerifier Verifier;
+  testImport("class declToImport { int DefInit = 5; }; declToImport X;",
+ Lang_CXX11, "", Lang_CXX11, Verifier,
+ cxxRecordDecl(hasDescendant(cxxConstructorDecl(
+ hasAnyConstructorInitializer(cxxCtorInitializer(
+ withInitializer(cxxDefaultInitExpr(;
+  testImport(
+  "struct X { int A = 5; }; X declToImport{};", Lang_CXX17, "", Lang_CXX17,
+  Verifier,
+  varDecl(hasInitializer(initListExpr(hasInit(0, cxxDefaultInitExpr());
+}
 
 const internal::VariadicDynCastAllOfMatcher vaArgExpr;
 
@@ -7529,6 +7544,92 @@
   EXPECT_TRUE(ToA->isCompleteDefinition());
 }
 
+TEST_P(ASTImporterOptionSpecificTestBase, ImportInClassInitializerFromField) {
+  // Encounter import of a field when the field already exists but has the
+  // in-class initializer expression not yet set. Such case can occur in the AST
+  // of generated template specializations.
+  // The first code forces to create a template specialization of
+  // `A` but without implicit constructors.
+  // The second ("From") code contains a variable of type `A`, this
+  // results in a template specialization that has constructors and
+  // CXXDefaultInitExpr nodes.
+  Decl *ToTU = getToTuDecl(
+  R"(
+  void f();
+  template struct A { int X = 1; };
+  struct B { A Y; };
+  )",
+  Lang_CXX11);
+  auto *ToX = FirstDeclMatcher().match(
+  ToTU,
+  fieldDecl(hasName("X"), hasParent(classTemplateSpecializationDecl(;
+  ASSERT_TRUE(ToX->hasInClassInitializer());
+  ASSERT_FALSE(ToX->getInClassInitializer());
+
+  Decl *FromTU = getTuDecl(
+  R"(
+  void f();
+  template struct A { int X = 1; };
+  struct B { A Y; };
+  //
+  A Z;
+  )",
+  Lang_CXX11, "input1.cc");
+  auto *FromX = FirstDeclMatcher().match(
+  FromTU,
+  fieldDecl(hasName("X"), hasParent(classTemplateSpecializationDecl(;
+
+  auto *ToXImported = Import(FromX, Lang_CXX11);
+  EXPECT_EQ(ToXImported, ToX);
+  EXPECT_TRUE(ToX->getInClassInitializer());
+}
+
+TEST_P(ASTImporterOptionSpecificTestBase,
+   ImportInClassInitializerFromCXXDefaultInitExpr) {
+  // Encounter AST import of a CXXDefaultInitExpr where the "to-field"
+  // of it exists but has the in-class initializer not set yet.
+  Decl *ToTU = getToTuDecl(
+  R"(
+  namespace N {
+template int b;
+struct X;
+  }
+  template struct A { N::X *X = nullptr; };
+  struct B { A Y; };
+  )",
+  Lang_CXX14);
+  auto *ToX = FirstDeclMatcher().match(
+  ToTU,
+  fieldDecl(hasName("X"), hasParent(classTemplateSpecializationDecl(;
+  ASSERT_TRUE(ToX->hasInClassInitializer());
+  ASSERT_FALSE(ToX->getInClassInitializer());
+
+  Decl *FromTU = getTuDecl(
+  R"(
+  namespace N {
+template int b;
+struct X;
+  }
+  template struct A { N::X *X = nullptr; };
+  struct B { A Y; };
+  //
+  void f() {
+(void)A{};
+  }
+  struct C {
+C(): attr(new A{}){}
+A *attr;
+const int value = N::b;
+  };
+  )",
+  Lang_CXX14, "input1.cc");
+  auto *FromF = FirstDeclMatcher().match(
+  FromTU, functionDecl(hasName("f"), isDefinition()));
+  auto *ToF = Import(FromF, Lang_CXX11);
+  EXPECT_TRUE(ToF);
+  EXPECT_TRUE(ToX->getInClassInitializer());
+}
+
 INSTANTIATE_TEST_SUITE_P(ParameterizedTests, ASTImporterLookupTableTest,
  DefaultTestValuesForRunOptions);
 
Index: clang/test/Analysis/ctu-cxxdefaultinitexpr.cpp
===
--- /dev/null
+++ clang/test/Analysis/ctu-cxxdefaultinitexpr.cpp
@@ -0,0 +1,35 @@
+// RUN: rm -rf %t && mkdir %t
+// RUN: mkdir -p %t/ctudir
+// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -std=c++17 \
+// RUN:   -emit-pch -o 

[PATCH] D122564: [RISCV] [NFC] add some tests for overloaded intrinsics of FP16

[Chenbing.Zheng via Phabricator via cfe-commits]

Chenbing.Zheng created this revision.
Chenbing.Zheng added reviewers: craig.topper, HsiangKai, benshi001, frasercrmck.
Chenbing.Zheng added a project: LLVM.
Herald added subscribers: s, VincentWu, luke957, StephenFan, vkmr, evandro, 
luismarques, apazos, sameer.abuasal, s.egerton, Jim, benna, psnobl, jocewei, 
PkmX, the_o, brucehoult, MartinMosbeck, rogfer01, edward-jones, zzheng, jrtc27, 
kito-cheng, niosHD, sabuasal, simoncook, johnrusso, rbar, asb, arichardson.
Herald added a project: All.
Chenbing.Zheng requested review of this revision.
Herald added subscribers: cfe-commits, pcwang-thead, eopXD, jacquesguan, 
MaskRay.
Herald added a project: clang.

These tests for overloaded intrinsics of FP16  is missing in vle/vse and 
vlse/vsse.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D122564

Files:
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vle.c
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vlse.c
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vse.c
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vsse.c

Index: clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vsse.c
===
--- clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vsse.c
+++ clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vsse.c
@@ -1,7 +1,7 @@
 // NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py
 // REQUIRES: riscv-registered-target
-// RUN: %clang_cc1 -triple riscv64 -target-feature +f -target-feature +d -target-feature +v \
-// RUN:   -disable-O0-optnone -emit-llvm %s -o - | opt -S -mem2reg | FileCheck --check-prefix=CHECK-RV64 %s
+// RUN: %clang_cc1 -triple riscv64 -target-feature +f -target-feature +d -target-feature +v -target-feature +zfh \
+// RUN:   -target-feature +experimental-zvfh -disable-O0-optnone -emit-llvm %s -o - | opt -S -mem2reg | FileCheck --check-prefix=CHECK-RV64 %s
 
 #include 
 
@@ -1170,3 +1170,123 @@
vfloat64m8_t value, size_t vl) {
   return vsse64(mask, base, bstride, value, vl);
 }
+
+// CHECK-RV64-LABEL: @test_vsse16_v_f16mf4(
+// CHECK-RV64-NEXT:  entry:
+// CHECK-RV64-NEXT:[[TMP0:%.*]] = bitcast half* [[BASE:%.*]] to *
+// CHECK-RV64-NEXT:call void @llvm.riscv.vsse.nxv1f16.i64( [[VALUE:%.*]], * [[TMP0]], i64 [[BSTRIDE:%.*]], i64 [[VL:%.*]])
+// CHECK-RV64-NEXT:ret void
+//
+void test_vsse16_v_f16mf4(_Float16 *base, ptrdiff_t bstride, vfloat16mf4_t value, size_t vl) {
+  return vsse16(base, bstride, value, vl);
+}
+
+// CHECK-RV64-LABEL: @test_vsse16_v_f16mf2(
+// CHECK-RV64-NEXT:  entry:
+// CHECK-RV64-NEXT:[[TMP0:%.*]] = bitcast half* [[BASE:%.*]] to *
+// CHECK-RV64-NEXT:call void @llvm.riscv.vsse.nxv2f16.i64( [[VALUE:%.*]], * [[TMP0]], i64 [[BSTRIDE:%.*]], i64 [[VL:%.*]])
+// CHECK-RV64-NEXT:ret void
+//
+void test_vsse16_v_f16mf2(_Float16 *base, ptrdiff_t bstride, vfloat16mf2_t value, size_t vl) {
+  return vsse16(base, bstride, value, vl);
+}
+
+// CHECK-RV64-LABEL: @test_vsse16_v_f16m1(
+// CHECK-RV64-NEXT:  entry:
+// CHECK-RV64-NEXT:[[TMP0:%.*]] = bitcast half* [[BASE:%.*]] to *
+// CHECK-RV64-NEXT:call void @llvm.riscv.vsse.nxv4f16.i64( [[VALUE:%.*]], * [[TMP0]], i64 [[BSTRIDE:%.*]], i64 [[VL:%.*]])
+// CHECK-RV64-NEXT:ret void
+//
+void test_vsse16_v_f16m1(_Float16 *base, ptrdiff_t bstride, vfloat16m1_t value, size_t vl) {
+  return vsse16(base, bstride, value, vl);
+}
+
+// CHECK-RV64-LABEL: @test_vsse16_v_f16m2(
+// CHECK-RV64-NEXT:  entry:
+// CHECK-RV64-NEXT:[[TMP0:%.*]] = bitcast half* [[BASE:%.*]] to *
+// CHECK-RV64-NEXT:call void @llvm.riscv.vsse.nxv8f16.i64( [[VALUE:%.*]], * [[TMP0]], i64 [[BSTRIDE:%.*]], i64 [[VL:%.*]])
+// CHECK-RV64-NEXT:ret void
+//
+void test_vsse16_v_f16m2(_Float16 *base, ptrdiff_t bstride, vfloat16m2_t value, size_t vl) {
+  return vsse16(base, bstride, value, vl);
+}
+
+// CHECK-RV64-LABEL: @test_vsse16_v_f16m4(
+// CHECK-RV64-NEXT:  entry:
+// CHECK-RV64-NEXT:[[TMP0:%.*]] = bitcast half* [[BASE:%.*]] to *
+// CHECK-RV64-NEXT:call void @llvm.riscv.vsse.nxv16f16.i64( [[VALUE:%.*]], * [[TMP0]], i64 [[BSTRIDE:%.*]], i64 [[VL:%.*]])
+// CHECK-RV64-NEXT:ret void
+//
+void test_vsse16_v_f16m4(_Float16 *base, ptrdiff_t bstride, vfloat16m4_t value, size_t vl) {
+  return vsse16(base, bstride, value, vl);
+}
+
+// CHECK-RV64-LABEL: @test_vsse16_v_f16m8(
+// CHECK-RV64-NEXT:  entry:
+// CHECK-RV64-NEXT:[[TMP0:%.*]] = bitcast half* [[BASE:%.*]] to *
+// CHECK-RV64-NEXT:call void @llvm.riscv.vsse.nxv32f16.i64( [[VALUE:%.*]], * [[TMP0]], i64 [[BSTRIDE:%.*]], i64 [[VL:%.*]])
+// CHECK-RV64-NEXT:ret void
+//
+void test_vsse16_v_f16m8(_Float16 *base, ptrdiff_t bstride, vfloat16m8_t value, size_t vl) {
+  return vsse16(base, bstride, value, vl);
+}
+
+// CHECK-RV64-LABEL: @test_vsse16_v_f16mf4_m(
+// CHECK-RV64-NEXT:  entry:
+// CHECK-RV64-NEXT:[[TMP0:%.*]] = bitcast half* [[BASE:%.*]] to *
+// CHECK-RV64-NEXT:call void @llvm.riscv.vsse.mas

[PATCH] D119409: [C++20] [Modules] Remain dynamic initializing internal-linkage variables in module interface unit

[Iain Sandoe via Phabricator via cfe-commits]

iains added a comment.

In D119409#3410474 , @ChuanqiXu wrote:

> In D119409#3409806 , @iains wrote:
>
>> I think that this problem might well be a consequence of the bug which is 
>> fixed by D122413 .
>>
>> We have been generating code with module internal entities (always) given 
>> the special ModuleInternalLinkage (which means that, although the linkage is 
>> formally 'internal', the entities are made global when emitted.  We should 
>> only be doing this for fmodules-ts, not for regular standard modules.
>>
>> If you apply D122413  (which I hope to 
>> land soon), then I would expect that iostream should work as expected (with 
>> one internal instance of std::__ioinit in each TU that includes iostream).
>>
>> IFF (after applying D122413  ) you add to 
>> the command line -fmodules-ts, then the patch here (D119409 
>> ) would, presumably, be needed to work 
>> around multiple instances of the globalised std::__ioinit.
>
> Sadly it wouldn't work after D122413  
> applied. Since the  is lived in GlobalModuleFragment, the 
> calculated linkage wouldn't affect them. So I met the same segfault as before.

Is this because we are not creating an initialiser for a static entity in the 
GMF ?

- I did a quick test and that seemed to be the case.

(module initialisers need quite some work, it seems)

>> addendum: note we still have work to do on the module initialisers - those 
>> are not correct yet (so probably some nesting of modules might not work).
>
> What does the nesting of modules mean?

If we have an import of a module that imports another - then we should be 
running the module initializers for the imported stack (in the correct order) 
.. at present, we do not do this.
As noted above, we have some work to do here.


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

https://reviews.llvm.org/D119409

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

[clang] 8a2a966 - Return -no-canonical-prefixes for riskv32/64 test

[Mikhail Goncharov via cfe-commits]

Author: Mikhail Goncharov
Date: 2022-03-28T11:23:14+02:00
New Revision: 8a2a966520023cd7d424e89b730c054f1f06f496

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

LOG: Return -no-canonical-prefixes for riskv32/64 test

W/o -no-canonical-prefixes CC1: clang{{.*}} "-cc1" "-triple" "riscv32"
does not match clang output for some setups.

See da62a5c6610dd2087ce2f527ca84ba43e93d5e30.

Added: 


Modified: 
clang/test/Driver/riscv32-toolchain.c
clang/test/Driver/riscv64-toolchain.c

Removed: 




diff  --git a/clang/test/Driver/riscv32-toolchain.c 
b/clang/test/Driver/riscv32-toolchain.c
index 7ab07563661c9..4eefa006c129f 100644
--- a/clang/test/Driver/riscv32-toolchain.c
+++ b/clang/test/Driver/riscv32-toolchain.c
@@ -1,6 +1,6 @@
 // A basic clang -cc1 command-line, and simple environment check.
 
-// RUN: %clang %s -### --target=riscv32 \
+// RUN: %clang %s -### -no-canonical-prefixes --target=riscv32 \
 // RUN:   --gcc-toolchain=%S/Inputs/basic_riscv32_tree 2>&1 \
 // RUN:   | FileCheck -check-prefix=CC1 %s
 // CC1: clang{{.*}} "-cc1" "-triple" "riscv32"

diff  --git a/clang/test/Driver/riscv64-toolchain.c 
b/clang/test/Driver/riscv64-toolchain.c
index 371dcfda4311a..d35e194031857 100644
--- a/clang/test/Driver/riscv64-toolchain.c
+++ b/clang/test/Driver/riscv64-toolchain.c
@@ -1,6 +1,6 @@
 // A basic clang -cc1 command-line, and simple environment check.
 
-// RUN: %clang %s -### --target=riscv64 \
+// RUN: %clang %s -### -no-canonical-prefixes --target=riscv64 \
 // RUN:   --gcc-toolchain=%S/Inputs/basic_riscv64_tree 2>&1 \
 // RUN:   | FileCheck -check-prefix=CC1 %s
 // CC1: clang{{.*}} "-cc1" "-triple" "riscv64"



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

[PATCH] D119409: [C++20] [Modules] Remain dynamic initializing internal-linkage variables in module interface unit

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu added a comment.

In D119409#3410868 , @iains wrote:

> In D119409#3410474 , @ChuanqiXu 
> wrote:
>
>> In D119409#3409806 , @iains wrote:
>>
>>> I think that this problem might well be a consequence of the bug which is 
>>> fixed by D122413 .
>>>
>>> We have been generating code with module internal entities (always) given 
>>> the special ModuleInternalLinkage (which means that, although the linkage 
>>> is formally 'internal', the entities are made global when emitted.  We 
>>> should only be doing this for fmodules-ts, not for regular standard modules.
>>>
>>> If you apply D122413  (which I hope to 
>>> land soon), then I would expect that iostream should work as expected (with 
>>> one internal instance of std::__ioinit in each TU that includes iostream).
>>>
>>> IFF (after applying D122413  ) you add to 
>>> the command line -fmodules-ts, then the patch here (D119409 
>>> ) would, presumably, be needed to work 
>>> around multiple instances of the globalised std::__ioinit.
>>
>> Sadly it wouldn't work after D122413  
>> applied. Since the  is lived in GlobalModuleFragment, the 
>> calculated linkage wouldn't affect them. So I met the same segfault as 
>> before.
>
> Is this because we are not creating an initialiser for a static entity in the 
> GMF ?
>
> - I did a quick test and that seemed to be the case.

I think we need this one finally, It would create the initialiser after the 
patch applied. And I think we couldn't do that without the patch. Since from 
the code we could see that the static variable wouldn't be generated in the 
current strategies.

> (module initialisers need quite some work, it seems)

The initialiser above I said is the initialiser in that TU. What you mean 
`module initializer` ? Do you mean the one module could have only module 
initializer?

>>> addendum: note we still have work to do on the module initialisers - those 
>>> are not correct yet (so probably some nesting of modules might not work).
>>
>> What does the nesting of modules mean?
>
> If we have an import of a module that imports another - then we should be 
> running the module initializers for the imported stack (in the correct order) 
> .. at present, we do not do this.
> As noted above, we have some work to do here.

I am not familiar with the history here. But I found 
http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2019/p1874r1.html#solution. 
It says clang already has a simple fix. So I am wondering if this one is 
already fixed or we are not talking about the same thing?


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

https://reviews.llvm.org/D119409

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

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

[Stanislav Gatev via Phabricator via cfe-commits]

sgatev added inline comments.



Comment at: 
clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp:270
+  // needed.
+  BoolValue &ComparisonValue = MakeValue(Env, *HasValueVal);
+  auto *ComparisonExprLoc =

ymandel wrote:
> ymandel wrote:
> > xazax.hun wrote:
> > > xazax.hun wrote:
> > > > ymandel wrote:
> > > > > xazax.hun wrote:
> > > > > > Is this the right way to initialize `ComparisonValue`?
> > > > > > 
> > > > > > Considering the expression: `opt.value_or(nullptr) != nullptr`
> > > > > > * When `has_value == false`, `opt.value_or(nullptr)` will return 
> > > > > > `nullptr`, so `!=` evaluates to false. This case seems to check out.
> > > > > > * However, when `has_value == true`, `opt` might still hold an 
> > > > > > `nullptr` and `!=` could still evaluate to false. 
> > > > > Thanks for digging into this. I think it's correct, but helpful to 
> > > > > step through:
> > > > > 
> > > > > Its correctness depends on `MakeValue`, so I'll focus on that in 
> > > > > particular. For the `nullptr` case, we'll get:
> > > > > ```
> > > > > HasValueVal && ContentsNotEqX
> > > > > ```
> > > > > So, when `has_value == true`, this basically reduces to 
> > > > > `ContentsNotEqX`. Since that's an atom, the result is indeterminate, 
> > > > > which I believe is the desired outcome.
> > > > > 
> > > > > WDYT?  Also, even if I've convinced you, please let me know how i can 
> > > > > improve the comments. For that matter, would `MakeValue` be better 
> > > > > with a more specific name, like "MakePredicate" or somesuch?
> > > > I think what confuses me is that we do something different for the 3 
> > > > cases. You convinced me that `HasValueVal && ContentsNotEqX` is 
> > > > correct. But we only do this for one branch out of the 3.  What is the 
> > > > reason for that?
> > > Oh, never mind. Yeah, I think changing `MakeValue` to `MakePredicate` 
> > > would make this a bit clearer. After a second read now I understand 
> > > better what is going on.
> > Just to be clear: the three cases you mean are lines 273-283, or something 
> > else?
> and never mind my question, then (I rpelied before I saw your updated). I'll 
> change the name and add comments.
Can you elaborate on the three cases on lines 273-283? Why not simply do

```
auto &ComparisonExprLoc = Env.createStorageLocation(*ComparisonExpr);
Env.setStorageLocation(ComparisonExpr, ComparisonExprLoc);
Env.setValue(ComparisonExprLoc, ComparisonValue);
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122231

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

[PATCH] D122567: [X86][AMX] enable amx cast intrinsics in FE.

[LuoYuanke via Phabricator via cfe-commits]

LuoYuanke created this revision.
Herald added a subscriber: pengfei.
Herald added a project: All.
LuoYuanke requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

We have some discission in D99152  and 
llvm-dev and finially come up with
a solution to add amx specific cast intrinsics. We've support the
intrinsics in llvm IR. This patch is to replace bitcast with amx cast
intrinsics in code emitting in FE.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D122567

Files:
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/test/CodeGen/X86/amx_api.c

Index: clang/test/CodeGen/X86/amx_api.c
===
--- clang/test/CodeGen/X86/amx_api.c
+++ clang/test/CodeGen/X86/amx_api.c
@@ -11,9 +11,11 @@
 // This is an example code and integration test.
 void test_api(int cond, short row, short col) {
   //CHECK-LABEL: @test_api
-  //CHECK: call x86_amx @llvm.x86.tileloadd64.internal
-  //CHECK: call x86_amx @llvm.x86.tdpbssd.internal
-  //CHECK: call void @llvm.x86.tilestored64.internal
+  //CHECK-DAG: call x86_amx @llvm.x86.tileloadd64.internal
+  //CHECK-DAG: call <256 x i32> @llvm.x86.cast.tile.to.vector.v256i32(x86_amx {{%.*}})
+  //CHECK-DAG: call x86_amx @llvm.x86.cast.vector.to.tile.v256i32(<256 x i32> {{%.*}})
+  //CHECK-DAG: call x86_amx @llvm.x86.tdpbssd.internal
+  //CHECK-DAG: call void @llvm.x86.tilestored64.internal
   __tile1024i a = {row, 8};
   __tile1024i b = {8, col};
   __tile1024i c = {row, col};
@@ -33,65 +35,70 @@
 
 void test_tile_loadd(short row, short col) {
   //CHECK-LABEL: @test_tile_loadd
-  //CHECK: call x86_amx @llvm.x86.tileloadd64.internal
-  //CHECK-NEXT: {{%.*}} = bitcast x86_amx {{%.*}} to <256 x i32>
+  //CHECK-DAG: call x86_amx @llvm.x86.tileloadd64.internal
+  //CHECK-DAG: call <256 x i32> @llvm.x86.cast.tile.to.vector.v256i32(x86_amx {{%.*}})
   __tile1024i a = {row, col};
   __tile_loadd(&a, buf, STRIDE);
 }
 
 void test_tile_stream_loadd(short row, short col) {
   //CHECK-LABEL: @test_tile_stream_loadd
-  //CHECK: call x86_amx @llvm.x86.tileloaddt164.internal
-  //CHECK-NEXT: {{%.*}} = bitcast x86_amx {{%.*}} to <256 x i32>
+  //CHECK-DAG: call x86_amx @llvm.x86.tileloaddt164.internal
+  //CHECK-DAG: call <256 x i32> @llvm.x86.cast.tile.to.vector.v256i32(x86_amx {{%.*}})
   __tile1024i a = {row, col};
   __tile_stream_loadd(&a, buf, STRIDE);
 }
 
 void test_tile_dpbssd(__tile1024i a, __tile1024i b, __tile1024i c) {
   //CHECK-LABEL: @test_tile_dpbssd
-  //CHECK: call x86_amx @llvm.x86.tdpbssd.internal
-  //CHECK-NEXT: {{%.*}} = bitcast x86_amx {{%.*}} to <256 x i32>
+  //CHECK-DAG: call x86_amx @llvm.x86.cast.vector.to.tile.v256i32(<256 x i32> {{%.*}})
+  //CHECK-DAG: call x86_amx @llvm.x86.tdpbssd.internal
+  //CHECK-DAG: call <256 x i32> @llvm.x86.cast.tile.to.vector.v256i32(x86_amx {{%.*}})
   __tile_dpbssd(&c, a, b);
 }
 
 void test_tile_dpbsud(__tile1024i a, __tile1024i b, __tile1024i c) {
   //CHECK-LABEL: @test_tile_dpbsud
-  //CHECK: call x86_amx @llvm.x86.tdpbsud.internal
-  //CHECK-NEXT: {{%.*}} = bitcast x86_amx {{%.*}} to <256 x i32>
+  //CHECK-DAG: call x86_amx @llvm.x86.cast.vector.to.tile.v256i32(<256 x i32> {{%.*}})
+  //CHECK-DAG: call x86_amx @llvm.x86.tdpbsud.internal
+  //CHECK-DAG: call <256 x i32> @llvm.x86.cast.tile.to.vector.v256i32(x86_amx {{%.*}})
   __tile_dpbsud(&c, a, b);
 }
 
 void test_tile_dpbusd(__tile1024i a, __tile1024i b, __tile1024i c) {
   //CHECK-LABEL: @test_tile_dpbusd
-  //CHECK: call x86_amx @llvm.x86.tdpbusd.internal
-  //CHECK-NEXT: {{%.*}} = bitcast x86_amx {{%.*}} to <256 x i32>
+  //CHECK-DAG: call x86_amx @llvm.x86.cast.vector.to.tile.v256i32(<256 x i32> {{%.*}})
+  //CHECK-DAG: call x86_amx @llvm.x86.tdpbusd.internal
+  //CHECK-DAG: call <256 x i32> @llvm.x86.cast.tile.to.vector.v256i32(x86_amx {{%.*}})
   __tile_dpbusd(&c, a, b);
 }
 
 void test_tile_dpbuud(__tile1024i a, __tile1024i b, __tile1024i c) {
   //CHECK-LABEL: @test_tile_dpbuud
-  //CHECK: call x86_amx @llvm.x86.tdpbuud.internal
-  //CHECK-NEXT: {{%.*}} = bitcast x86_amx {{%.*}} to <256 x i32>
+  //CHECK-DAG: call x86_amx @llvm.x86.cast.vector.to.tile.v256i32(<256 x i32> {{%.*}})
+  //CHECK-DAG: call x86_amx @llvm.x86.tdpbuud.internal
+  //CHECK-DAG: call <256 x i32> @llvm.x86.cast.tile.to.vector.v256i32(x86_amx {{%.*}})
   __tile_dpbuud(&c, a, b);
 }
 
 void test_tile_stored(__tile1024i c) {
   //CHECK-LABEL: @test_tile_stored
-  //CHECK: {{%.*}} = bitcast <256 x i32> {{%.*}} to x86_amx
-  //CHECK-NEXT: call void @llvm.x86.tilestored64.internal
+  //CHECK-DAG: call x86_amx @llvm.x86.cast.vector.to.tile.v256i32(<256 x i32> {{%.*}})
+  //CHECK-DAG: call void @llvm.x86.tilestored64.internal
   __tile_stored(buf, STRIDE, c);
 }
 
 void test_tile_zero(__tile1024i c) {
   //CHECK-LABEL: @test_tile_zero
-  //CHECK: call x86_amx @llvm.x86.tilezero.internal
-  //CHECK-NEXT bitcast x86_amx {{%.*}} to <256 x i32>
+  //CHECK-DAG: ca

[PATCH] D99152: [AMX] Prototype for vector and amx bitcast.

[LuoYuanke via Phabricator via cfe-commits]

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

In D99152#3235546 , @lebedev.ri wrote:

> What's the status here?

Here is the patch D122567 .


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99152

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

[PATCH] D122554: [clangd] Handle tabs in getIncrementalChangesAfterNewline()

[Sam McCall via Phabricator via cfe-commits]

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

Thanks!

Going to add tabs to case-sensitivity and symlinks to my list of things that 
will haunt me forever...




Comment at: clang-tools-extra/clangd/Format.cpp:127
+  int ContentWidth = llvm::sys::unicode::columnWidthUTF8(Text);
+  if (ContentWidth >= 0)
+return ContentWidth;

Comment "unprintable characters" or so?
(Maybe invert the condition for this purpose)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122554

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

[PATCH] D122469: OpenMP 5.1 - Support 'seq_cst' clause on 'flush' directive

[Soumitra Chatterjee via Phabricator via cfe-commits]

soumitra updated this revision to Diff 418530.
soumitra added a comment.

The previous patch had unrelated changes inadvertently. Corrected.


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

https://reviews.llvm.org/D122469

Files:
  clang/include/clang/AST/OpenMPClause.h
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaOpenMP.cpp
  clang/test/OpenMP/flush_ast_print.cpp
  clang/test/OpenMP/flush_codegen.cpp
  clang/test/OpenMP/flush_messages.cpp
  llvm/include/llvm/Frontend/OpenMP/OMP.td

Index: llvm/include/llvm/Frontend/OpenMP/OMP.td
===
--- llvm/include/llvm/Frontend/OpenMP/OMP.td
+++ llvm/include/llvm/Frontend/OpenMP/OMP.td
@@ -517,6 +517,7 @@
 VersionedClause,
 VersionedClause,
 VersionedClause,
+VersionedClause,
 // TODO This should ne `none` instead. Comment carried over from
 // OMPKinds.def.
 VersionedClause
Index: clang/test/OpenMP/flush_messages.cpp
===
--- clang/test/OpenMP/flush_messages.cpp
+++ clang/test/OpenMP/flush_messages.cpp
@@ -1,8 +1,8 @@
 // RUN: %clang_cc1 -verify=expected,omp45 -fopenmp -fopenmp-version=45 -ferror-limit 100 %s -Wuninitialized
-// RUN: %clang_cc1 -verify=expected,omp50 -fopenmp -ferror-limit 100 %s -Wuninitialized
+// RUN: %clang_cc1 -verify=expected,omp51 -fopenmp -ferror-limit 100 %s -Wuninitialized
 
 // RUN: %clang_cc1 -verify=expected,omp45 -fopenmp-simd -fopenmp-version=45 -ferror-limit 100 %s -Wuninitialized
-// RUN: %clang_cc1 -verify=expected,omp50 -fopenmp-simd -ferror-limit 100 %s -Wuninitialized
+// RUN: %clang_cc1 -verify=expected,omp51 -fopenmp-simd -ferror-limit 100 %s -Wuninitialized
 
 struct S1 { // expected-note 2 {{declared here}}
   int a;
@@ -140,9 +140,12 @@
 #pragma omp flush release // omp45-error {{unexpected OpenMP clause 'release' in directive '#pragma omp flush'}}
 #pragma omp flush relaxed // expected-error {{unexpected OpenMP clause 'relaxed' in directive '#pragma omp flush'}}
 #pragma omp flush seq_cst // expected-error {{unexpected OpenMP clause 'seq_cst' in directive '#pragma omp flush'}}
-#pragma omp flush acq_rel acquire // omp45-error {{unexpected OpenMP clause 'acq_rel' in directive '#pragma omp flush'}} omp45-error {{unexpected OpenMP clause 'acquire' in directive '#pragma omp flush'}} omp50-error {{directive '#pragma omp flush' cannot contain more than one 'acq_rel', 'acquire' or 'release' clause}} omp50-note {{'acq_rel' clause used here}}
-#pragma omp flush release acquire // omp45-error {{unexpected OpenMP clause 'release' in directive '#pragma omp flush'}} omp45-error {{unexpected OpenMP clause 'acquire' in directive '#pragma omp flush'}} omp50-error {{directive '#pragma omp flush' cannot contain more than one 'acq_rel', 'acquire' or 'release' clause}} omp50-note {{'release' clause used here}}
+#pragma omp flush acq_rel acquire // omp45-error {{unexpected OpenMP clause 'acq_rel' in directive '#pragma omp flush'}} omp45-error {{unexpected OpenMP clause 'acquire' in directive '#pragma omp flush'}} omp50-error {{directive '#pragma omp flush' cannot contain more than one 'seq_cst', 'acq_rel', 'acquire' or 'release' clause}} omp50-note {{'acq_rel' clause used here}} omp51-error {{directive '#pragma omp flush' cannot contain more than one 'seq_cst', 'acq_rel', 'acquire' or 'release' clause}} omp51-note {{'acq_rel' clause used here}}
+
+#pragma omp flush release acquire // omp45-error {{unexpected OpenMP clause 'release' in directive '#pragma omp flush'}} omp45-error {{unexpected OpenMP clause 'acquire' in directive '#pragma omp flush'}} omp50-error {{directive '#pragma omp flush' cannot contain more than one 'seq_cst', 'acq_rel', 'acquire' or 'release' clause}} omp50-note {{'release' clause used here}} omp51-error {{directive '#pragma omp flush' cannot contain more than one 'seq_cst', 'acq_rel', 'acquire' or 'release' clause}} omp51-note {{'release' clause used here}}
+
 #pragma omp flush acq_rel (argc) // omp45-error {{unexpected OpenMP clause 'acq_rel' in directive '#pragma omp flush'}} expected-warning {{extra tokens at the end of '#pragma omp flush' are ignored}}
-#pragma omp flush(argc) acq_rel // omp45-error {{unexpected OpenMP clause 'acq_rel' in directive '#pragma omp flush'}} omp50-error {{'flush' directive with memory order clause 'acq_rel' cannot have the list}} omp50-note {{memory order clause 'acq_rel' is specified here}}
+#pragma omp flush(argc) acq_rel // omp45-error {{unexpected OpenMP clause 'acq_rel' in directive '#pragma omp flush'}} omp50-error {{'flush' directive with memory order clause 'acq_rel' cannot have the list}} omp50-note {{memory order clause 'acq_rel' is specified here}} omp51-error {{'flush' directive with memory order clause 'acq_rel' cannot have the list}} omp51-note {{memory order clause 'acq_rel' is specified here}}
+
   return tmain(argc);
 }
Index: clang/test/OpenMP/flush_codegen.cpp
=

[PATCH] D121451: [clang-format] Add space to comments starting with '#'.

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

In D121451#3402167 , @curdeius wrote:

> You comment the whole block and it becomes (with this patch):
>
>   // #include 
>   //
>   // int something;
>
> whereas it was:
>
>   //#include 
>   //
>   // int something;
>
> which seems inconsistent at least.

A fun wrinkle here is is that if you originally wrote (note no blank line):

  //#include 
  //int something;

and ran old clang-format, you get (and checked in)

  //#include 
  // int something;

and now if you run new clang-format, the second line gets an *extra* space to 
preserve the indent:

  // #include 
  //  int something;

I don't think this is worth fixing, had me scratching my head though :-)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121451

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

[PATCH] D54943: [clang-tidy] implement const-transformation for cppcoreguidelines-const-correctness

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added a comment.

ok. then i will continue under `misc-const-correctness`. thank you for the 
clarifications!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D54943

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

[PATCH] D122547: [clang] Make Driver tests pass when running with temp dir containing "crt"

[Hans Wennborg via Phabricator via cfe-commits]

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

lgtm


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

https://reviews.llvm.org/D122547

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

[PATCH] D91000: [clang-tidy] Add bugprone-unsafe-functions checker.

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

balazske added a comment.

I found only small issues.




Comment at: clang-tools-extra/clang-tidy/bugprone/UnsafeFunctionsCheck.cpp:109
+
+  SourceRange Range = SourceRange(DeclRef->getBeginLoc(), 
DeclRef->getEndLoc());
+  // FIXME: I'm not sure if this can ever happen, but to be on the safe side,

`DeclRef->getSourceRange()` can be used



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/bugprone-usafe-functions.rst:4
+bugprone-unsafe-functions
+
+

I do not know if this works, it is better if this line has the same length as 
the line above.



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/bugprone-usafe-functions.rst:105
+
+if (strcat_s(buf, suffix) != 0) {
+  // error handling

These examples do not work: `strcat_s` and `strcpy_s` take different parameters 
than the original.


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

https://reviews.llvm.org/D91000

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

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

[Kai Luo via Phabricator via cfe-commits]

lkail updated this revision to Diff 418537.
lkail added a comment.

Thanks @efriedma for pointing out. Updated guard code. Also adjust backend to 
reflect ABI change.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122377

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

Index: llvm/test/CodeGen/PowerPC/atomics-i128.ll
===
--- llvm/test/CodeGen/PowerPC/atomics-i128.ll
+++ llvm/test/CodeGen/PowerPC/atomics-i128.ll
@@ -5,6 +5,15 @@
 ; RUN: llc -verify-machineinstrs -mtriple=powerpc64-unknown-unknown -mcpu=pwr7 \
 ; RUN:   -ppc-asm-full-reg-names -ppc-quadword-atomics \
 ; RUN:   -ppc-track-subreg-liveness < %s | FileCheck --check-prefix=PWR7 %s
+; RUN: llc -verify-machineinstrs -mtriple=powerpc64le-unknown-linux-gnu -mcpu=pwr8 \
+; RUN:   -ppc-asm-full-reg-names -ppc-track-subreg-liveness < %s | FileCheck \
+; RUN:   --check-prefix=LE-PWR8 %s
+; RUN: llc -verify-machineinstrs -mtriple=powerpc64le-unknown-freebsd -mcpu=pwr8 \
+; RUN:   -ppc-asm-full-reg-names -ppc-track-subreg-liveness < %s | FileCheck \
+; RUN:   --check-prefix=LE-PWR8 %s
+; RUN: llc -verify-machineinstrs -mtriple=powerpc64-unknown-aix -mcpu=pwr8 \
+; RUN:   -ppc-asm-full-reg-names -ppc-track-subreg-liveness < %s | FileCheck \
+; RUN:   --check-prefix=AIX64-PWR8 %s
 
 
 define i128 @swap(i128* %a, i128 %x) {
@@ -39,6 +48,36 @@
 ; PWR7-NEXT:ld r0, 16(r1)
 ; PWR7-NEXT:mtlr r0
 ; PWR7-NEXT:blr
+;
+; LE-PWR8-LABEL: swap:
+; LE-PWR8:   # %bb.0: # %entry
+; LE-PWR8-NEXT:sync
+; LE-PWR8-NEXT:  .LBB0_1: # %entry
+; LE-PWR8-NEXT:#
+; LE-PWR8-NEXT:lqarx r6, 0, r3
+; LE-PWR8-NEXT:mr r9, r4
+; LE-PWR8-NEXT:mr r8, r5
+; LE-PWR8-NEXT:stqcx. r8, 0, r3
+; LE-PWR8-NEXT:bne cr0, .LBB0_1
+; LE-PWR8-NEXT:  # %bb.2: # %entry
+; LE-PWR8-NEXT:lwsync
+; LE-PWR8-NEXT:mr r3, r7
+; LE-PWR8-NEXT:mr r4, r6
+; LE-PWR8-NEXT:blr
+;
+; AIX64-PWR8-LABEL: swap:
+; AIX64-PWR8:   # %bb.0: # %entry
+; AIX64-PWR8-NEXT:mflr r0
+; AIX64-PWR8-NEXT:std r0, 16(r1)
+; AIX64-PWR8-NEXT:stdu r1, -112(r1)
+; AIX64-PWR8-NEXT:sync
+; AIX64-PWR8-NEXT:bl .__sync_lock_test_and_set_16[PR]
+; AIX64-PWR8-NEXT:nop
+; AIX64-PWR8-NEXT:lwsync
+; AIX64-PWR8-NEXT:addi r1, r1, 112
+; AIX64-PWR8-NEXT:ld r0, 16(r1)
+; AIX64-PWR8-NEXT:mtlr r0
+; AIX64-PWR8-NEXT:blr
 entry:
   %0 = atomicrmw xchg i128* %a, i128 %x seq_cst, align 16
   ret i128 %0
@@ -76,6 +115,36 @@
 ; PWR7-NEXT:ld r0, 16(r1)
 ; PWR7-NEXT:mtlr r0
 ; PWR7-NEXT:blr
+;
+; LE-PWR8-LABEL: add:
+; LE-PWR8:   # %bb.0: # %entry
+; LE-PWR8-NEXT:sync
+; LE-PWR8-NEXT:  .LBB1_1: # %entry
+; LE-PWR8-NEXT:#
+; LE-PWR8-NEXT:lqarx r6, 0, r3
+; LE-PWR8-NEXT:addc r9, r4, r7
+; LE-PWR8-NEXT:adde r8, r5, r6
+; LE-PWR8-NEXT:stqcx. r8, 0, r3
+; LE-PWR8-NEXT:bne cr0, .LBB1_1
+; LE-PWR8-NEXT:  # %bb.2: # %entry
+; LE-PWR8-NEXT:lwsync
+; LE-PWR8-NEXT:mr r3, r7
+; LE-PWR8-NEXT:mr r4, r6
+; LE-PWR8-NEXT:blr
+;
+; AIX64-PWR8-LABEL: add:
+; AIX64-PWR8:   # %bb.0: # %entry
+; AIX64-PWR8-NEXT:mflr r0
+; AIX64-PWR8-NEXT:std r0, 16(r1)
+; AIX64-PWR8-NEXT:stdu r1, -112(r1)
+; AIX64-PWR8-NEXT:sync
+; AIX64-PWR8-NEXT:bl .__sync_fetch_and_add_16[PR]
+; AIX64-PWR8-NEXT:nop
+; AIX64-PWR8-NEXT:lwsync
+; AIX64-PWR8-NEXT:addi r1, r1, 112
+; AIX64-PWR8-NEXT:ld r0, 16(r1)
+; AIX64-PWR8-NEXT:mtlr r0
+; AIX64-PWR8-NEXT:blr
 entry:
   %0 = atomicrmw add i128* %a, i128 %x seq_cst, align 16
   ret i128 %0
@@ -113,6 +182,36 @@
 ; PWR7-NEXT:ld r0, 16(r1)
 ; PWR7-NEXT:mtlr r0
 ; PWR7-NEXT:blr
+;
+; LE-PWR8-LABEL: sub:
+; LE-PWR8:   # %bb.0: # %entry
+; LE-PWR8-NEXT:sync
+; LE-PWR8-NEXT:  .LBB2_1: # %entry
+; LE-PWR8-NEXT:#
+; LE-PWR8-NEXT:lqarx r6, 0, r3
+; LE-PWR8-NEXT:subc r9, r7, r4
+; LE-PWR8-NEXT:subfe r8, r5, r6
+; LE-PWR8-NEXT:stqcx. r8, 0, r3
+; LE-PWR8-NEXT:bne cr0, .LBB2_1
+; LE-PWR8-NEXT:  # %bb.2: # %entry
+; LE-PWR8-NEXT:lwsync
+; LE-PWR8-NEXT:mr r3, r7
+; LE-PWR8-NEXT:mr r4, r6
+; LE-PWR8-NEXT:blr
+;
+; AIX64-PWR8-LABEL: sub:
+; AIX64-PWR8:   # %bb.0: # %entry
+; AIX64-PWR8-NEXT:mflr r0
+; AIX64-PWR8-NEXT:std r0, 16(r1)
+; AIX64-PWR8-NEXT:stdu r1, -112(r1)
+; AIX64-PWR8-NEXT:sync
+; AIX64-PWR8-NEXT:bl .__sync_fetch_and_sub_16[PR]
+; AIX64-PWR8-NEXT:nop
+; AIX64-PWR8-NEXT:lwsync
+; AIX64-PWR8-NEXT:addi r1, r1, 112
+; AIX64-PWR8-NEXT:ld r0, 16(r1)
+; AIX64-PWR8-NEXT:mtlr r0
+; AIX64-PWR8-NEXT:blr
 entry:
   %0 = atomicrmw sub i128* %a,

[PATCH] D122495: [clang][extract-api] Use correct language info from inputs

[Daniel Grumberg via Phabricator via cfe-commits]

dang added inline comments.



Comment at: clang/include/clang/ExtractAPI/API.h:312
 
-  /// Get the language options used to parse the APIs.
-  const LangOptions &getLangOpts() const { return LangOpts; }
+  /// Get the language by the APIs.
+  Language getLanguage() const { return Lang; }

Nit: "by" isn't great here, maybe something along the lines of "Get the 
language the APIs are defined in."



Comment at: clang/include/clang/ExtractAPI/API.h:341
   const llvm::Triple Target;
-  const LangOptions LangOpts;
+  Language Lang;
 

`const` ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122495

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

[PATCH] D122573: [TBAA] Emit distinct TBAA tags for pointers with different depths,types.

[Florian Hahn via Phabricator via cfe-commits]

fhahn created this revision.
fhahn added reviewers: rjmccall, aaron.ballman, scanon, rsmith, hfinkel.
Herald added subscribers: jeroen.dobbelaere, kosarev.
Herald added a project: All.
fhahn requested review of this revision.
Herald added a project: clang.

This patch extends Clang's TBAA generation code to emit distinct tags
for incompatible pointer types.

Pointers with different element types are incompatible if the pointee
types are also incompatible (modulo sugar/modifiers).

Express this in TBAA by generating different tags for pointers based
on the pointer depth and pointee type. To get the TBAA tag for the
pointee type it uses getTypeInfoHelper on the pointee type.

I'd appreciate a careful review,  because this is probably quite easy to
get subtly wrong.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D122573

Files:
  clang/lib/CodeGen/CodeGenTBAA.cpp
  clang/test/CodeGen/tbaa-pointers.c

Index: clang/test/CodeGen/tbaa-pointers.c
===
--- clang/test/CodeGen/tbaa-pointers.c
+++ clang/test/CodeGen/tbaa-pointers.c
@@ -1,79 +1,79 @@
 // RUN: %clang_cc1 -triple x86_64-apple-darwin -O1 -disable-llvm-passes %s -emit-llvm -o - | FileCheck %s
 
-void p2unsigned(unsigned **ptr) {
+void p2unsigned(unsigned** ptr) {
   // CHECK-LABEL: define void @p2unsigned(i32** noundef %ptr)
   // CHECK-NEXT: entry:
   // CHECK-NEXT:  %ptr.addr = alloca i32**, align 8
-  // CHECK-NEXT:  store i32** %ptr, i32*** %ptr.addr, align 8, !tbaa [[ANY_POINTER_0:!.+]]
-  // CHECK-NEXT:  [[BASE:%.+]] = load i32**, i32*** %ptr.addr, align 8, !tbaa [[ANY_POINTER_0]]
-  // CHECK-NEXT:  store i32* null, i32** [[BASE]], align 8, !tbaa [[ANY_POINTER_0]]
+  // CHECK-NEXT:  store i32** %ptr, i32*** %ptr.addr, align 8, !tbaa [[P2INT_0:!.+]]
+  // CHECK-NEXT:  [[BASE:%.+]] = load i32**, i32*** %ptr.addr, align 8, !tbaa [[P2INT_0]]
+  // CHECK-NEXT:  store i32* null, i32** [[BASE]], align 8, !tbaa [[P1INT_0:!.+]]
   // CHECK-NEXT:  ret void
   //
   *ptr = 0;
 }
 
-void p2unsigned_volatile(unsigned *volatile *ptr) {
+void p2unsigned_volatile(unsigned* volatile* ptr) {
   // CHECK-LABEL: define void @p2unsigned_volatile(i32** noundef %ptr)
   // CHECK-NEXT: entry:
   // CHECK-NEXT:   %ptr.addr = alloca i32**, align 8
-  // CHECK-NEXT:   store i32** %ptr, i32*** %ptr.addr, align 8, !tbaa [[ANY_POINTER_0]]
-  // CHECK-NEXT:   [[BASE:%.+]] = load i32**, i32*** %ptr.addr, align 8, !tbaa [[ANY_POINTER_0]]
-  // CHECK-NEXT:   store volatile i32* null, i32** [[BASE]], align 8, !tbaa [[ANY_POINTER_0]]
+  // CHECK-NEXT:   store i32** %ptr, i32*** %ptr.addr, align 8, !tbaa [[P2INT_0]]
+  // CHECK-NEXT:   [[BASE:%.+]] = load i32**, i32*** %ptr.addr, align 8, !tbaa [[P2INT_0]]
+  // CHECK-NEXT:   store volatile i32* null, i32** [[BASE]], align 8, !tbaa [[P1INT_0]]
   // CHECK-NEXT:   ret void
   //
   *ptr = 0;
 }
 
-void p3int(int ***ptr) {
+void p3int(int*** ptr) {
   // CHECK-LABEL: define void @p3int(i32*** noundef %ptr)
   // CHECK-NEXT: entry:
   // CHECK-NEXT:   %ptr.addr = alloca i32***, align 8
-  // CHECK-NEXT:   store i32*** %ptr, i32 %ptr.addr, align 8, !tbaa [[ANY_POINTER_0]]
-  // CHECK-NEXT:   [[BASE_0:%.+]] = load i32***, i32 %ptr.addr, align 8, !tbaa [[ANY_POINTER_0]]
-  // CHECK-NEXT:   [[BASE_1:%.+]] = load i32**, i32*** [[BASE_0]], align 8, !tbaa [[ANY_POINTER_0]]
-  // CHECK-NEXT:   store i32* null, i32** [[BASE_1]], align 8, !tbaa [[ANY_POINTER_0]]
+  // CHECK-NEXT:   store i32*** %ptr, i32 %ptr.addr, align 8, !tbaa [[P3INT_0:!.+]]
+  // CHECK-NEXT:   [[BASE_0:%.+]] = load i32***, i32 %ptr.addr, align 8, !tbaa [[P3INT_0]]
+  // CHECK-NEXT:   [[BASE_1:%.+]] = load i32**, i32*** [[BASE_0]], align 8, !tbaa [[P2INT_0]]
+  // CHECK-NEXT:   store i32* null, i32** [[BASE_1]], align 8, !tbaa [[P1INT_0]]
   // CHECK-NEXT:   ret void
   //
   **ptr = 0;
 }
 
-void p4char(char ptr) {
+void p4char(char ptr) {
   // CHECK-LABEL: define void @p4char(i8 noundef %ptr)
   // CHECK-NEXT: entry:
   // CHECK-NEXT:   %ptr.addr = alloca i8, align 8
-  // CHECK-NEXT:   store i8 %ptr, i8* %ptr.addr, align 8, !tbaa [[ANY_POINTER_0]]
-  // CHECK-NEXT:   [[BASE_0:%.+]] = load i8, i8* %ptr.addr, align 8, !tbaa [[ANY_POINTER_0]]
-  // CHECK-NEXT:   [[BASE_1:%.+]] = load i8***, i8 [[BASE_0]], align 8, !tbaa [[ANY_POINTER_0]]
-  // CHECK-NEXT:   [[BASE_2:%.+]] = load i8**, i8*** [[BASE_1]], align 8, !tbaa [[ANY_POINTER_0]]
-  // CHECK-NEXT:   store i8* null, i8** [[BASE_2]], align 8, !tbaa [[ANY_POINTER_0]]
+  // CHECK-NEXT:   store i8 %ptr, i8* %ptr.addr, align 8, !tbaa [[P4CHAR_0:!.+]]
+  // CHECK-NEXT:   [[BASE_0:%.+]] = load i8, i8* %ptr.addr, align 8, !tbaa [[P4CHAR_0]]
+  // CHECK-NEXT:   [[BASE_1:%.+]] = load i8***, i8 [[BASE_0]], align 8, !tbaa [[P3CHAR_0:!.+]]
+  // CHECK-NEXT:   [[BASE_2:%.+]] = load i8**, i8*** [[BASE_1]], align 8, !tbaa [[P2CHAR_0:!.+]]
+  // CHECK-NEXT:   store i8* null, i8** [[BASE_2]

[PATCH] D122234: [clang] Link libbitint for large division of _BitInt; increase max _BitInt size

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:8422
 
+def err_int_to_float_bit_int_max_size : Error<
+  "cannot convert '_BitInt' operands of more than %0 bits to floating point">;

mgehre-amd wrote:
> aaron.ballman wrote:
> > mgehre-amd wrote:
> > > aaron.ballman wrote:
> > > > mgehre-amd wrote:
> > > > > erichkeane wrote:
> > > > > > Can you explain the issue here?  This is supposed to be 
> > > > > > well-defined behavior.
> > > > > I saw `Assertion LC != RTLIB::UNKNOWN_LIBCALL && "Don't know how to 
> > > > > expand this SINT_TO_FP!"' failed.` in the backend. I think we would 
> > > > > need to add library calls for floating to bitint (and vice versa) to 
> > > > > the bitint library to enable that.
> > > > Good catch! I think we'll need to solve that before we can enable wide 
> > > > bit-width support (users are going to expect assignment and 
> > > > initialization to not crash as those are basic builtin operators). 
> > > > FWIW, this is a reproducer from Clang 13 where we still allowed large 
> > > > widths: https://godbolt.org/z/hvzWq1KTK
> > > I don't think I agree. 
> > > 
> > > Right now users will get a diagnostic that _BitInt > 128 bit cannot be 
> > > used. With this PR, they can use them but get a diagnostic when they try 
> > > to convert large _BitInts to floats or back.
> > > I think there is huge value in allowing large _BitInts even when float 
> > > to/from conversion will cause a clang diagnostic because at least in my 
> > > use case that is pretty uncommon,
> > > so I propose to move forward with this PR (lifting the limit on _BitInt 
> > > width) without having the compiler-rt support for float conversions.
> > > Right now users will get a diagnostic that _BitInt > 128 bit cannot be 
> > > used. With this PR, they can use them but get a diagnostic when they try 
> > > to convert large _BitInts to floats or back.
> > 
> > This is certainly better than crashing, no doubt.
> > 
> > > I think there is huge value in allowing large _BitInts even when float 
> > > to/from conversion will cause a clang diagnostic because at least in my 
> > > use case that is pretty uncommon,
> > 
> > That's the problem -- this is a new basic datatype; we can't make 
> > assumptions that our constituent uses cases are the common pattern. My 
> > experience thus far with this feature has been that people are using it for 
> > all sorts of reasons in ways I wouldn't have expected, like as a big int, 
> > as a regular int that doesn't integer promote, as bit-fields, etc.
> > 
> > To me, the bar for whether we allow large bit widths than we do today is: 
> > do all basic mathematical operators on any bit-width work correctly at 
> > runtime without crashing or major compile-time or runtime performance 
> > issues for the given target you want to change the limit for? Assignment 
> > and conversion are basic mathematical operators I'd definitely expect to 
> > work; these aren't weird situations -- assigning a float to an integer 
> > happens with some regularity, so there's no reason to assume it won't 
> > happen when the integer is a larger bit-precise integer: 
> > https://godbolt.org/z/hx5sYbjGK.
> > 
> > I'd *much* rather slow-roll the feature than have people's first impression 
> > be that they can't trust the feature because it's too flaky. The whole 
> > point to the `BITINT_MAXWIDTH` macro is to give the user a reliable way to 
> > know what bit widths are supported; lying will not do us any favors.
> Ok, then let me propose the following:
> - Keep the default max. _BitInt size limit at 128 with this PR
> - Remove the code that emits the float-to-int/int-to-float warnings for bit 
> _BitInts
> - Add a cc1 option -fmax-bitint-size=N to allow overwriting the max. _BitInt 
> size for tests (so we can test big division even when float-to-int isn't 
> there yet)
> 
> What do you think?
> 
I was hoping to avoid something like that, but I think it's not entirely 
unreasonable as a stopgap measure either.

How about we name it `-fexperimental-max-bitint-width=N` to make it clear that 
this option is not expected to stick around forever? (I'd like to not ship a 
release of Clang with this flag). I suppose we'd still have to make sure the 
user doesn't specify a value larger than what LLVM supports, too.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122234

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

[PATCH] D122478: [PowerPC] Add max/min intrinsics to Clang and PPC backend

[Ting Wang via Phabricator via cfe-commits]

tingwang updated this revision to Diff 418544.
tingwang marked 11 inline comments as done.
tingwang added a comment.

Update based on Chaofan's suggestions.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122478

Files:
  clang/include/clang/Basic/BuiltinsPPC.def
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Basic/Targets/PPC.cpp
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/Sema/SemaChecking.cpp
  clang/test/CodeGen/PowerPC/builtins-ppc.c
  clang/test/Sema/builtins-ppc.c
  llvm/include/llvm/IR/IntrinsicsPowerPC.td
  llvm/lib/Target/PowerPC/PPCISelLowering.cpp
  llvm/test/CodeGen/PowerPC/builtins-ppc-xlcompat-maxmin.ll

Index: llvm/test/CodeGen/PowerPC/builtins-ppc-xlcompat-maxmin.ll
===
--- /dev/null
+++ llvm/test/CodeGen/PowerPC/builtins-ppc-xlcompat-maxmin.ll
@@ -0,0 +1,257 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc -mcpu=pwr9 -verify-machineinstrs -mtriple=powerpc64le-unknown-linux \
+; RUN:< %s | FileCheck --check-prefixes=CHECK,CHECK-P9 %s
+; RUN: llc -mcpu=pwr8 -verify-machineinstrs -mtriple=powerpc64le-unknown-linux \
+; RUN:< %s | FileCheck --check-prefixes=CHECK,CHECK-P8 %s
+
+declare ppc_fp128 @llvm.ppc.maxfe(ppc_fp128 %a, ppc_fp128 %b, ppc_fp128 %c, ...)
+define ppc_fp128 @test_maxfe(ppc_fp128 %a, ppc_fp128 %b, ppc_fp128 %c, ppc_fp128 %d) {
+; CHECK-LABEL: test_maxfe:
+; CHECK:   # %bb.0: # %entry
+; CHECK-NEXT:fcmpu 0, 6, 4
+; CHECK-NEXT:fcmpu 1, 5, 3
+; CHECK-NEXT:crand 20, 6, 1
+; CHECK-NEXT:cror 20, 5, 20
+; CHECK-NEXT:bc 12, 20, .LBB0_2
+; CHECK-NEXT:  # %bb.1: # %entry
+; CHECK-NEXT:fmr 6, 4
+; CHECK-NEXT:  .LBB0_2: # %entry
+; CHECK-NEXT:fcmpu 0, 6, 2
+; CHECK-NEXT:bc 12, 20, .LBB0_4
+; CHECK-NEXT:  # %bb.3: # %entry
+; CHECK-NEXT:fmr 5, 3
+; CHECK-NEXT:  .LBB0_4: # %entry
+; CHECK-NEXT:fcmpu 1, 5, 1
+; CHECK-NEXT:crand 20, 6, 1
+; CHECK-NEXT:cror 20, 5, 20
+; CHECK-NEXT:bc 12, 20, .LBB0_6
+; CHECK-NEXT:  # %bb.5: # %entry
+; CHECK-NEXT:fmr 6, 2
+; CHECK-NEXT:  .LBB0_6: # %entry
+; CHECK-NEXT:fcmpu 0, 6, 8
+; CHECK-NEXT:bc 12, 20, .LBB0_8
+; CHECK-NEXT:  # %bb.7: # %entry
+; CHECK-NEXT:fmr 5, 1
+; CHECK-NEXT:  .LBB0_8: # %entry
+; CHECK-NEXT:fcmpu 1, 5, 7
+; CHECK-NEXT:crand 20, 6, 1
+; CHECK-NEXT:cror 20, 5, 20
+; CHECK-NEXT:bc 12, 20, .LBB0_10
+; CHECK-NEXT:  # %bb.9: # %entry
+; CHECK-NEXT:fmr 5, 7
+; CHECK-NEXT:  .LBB0_10: # %entry
+; CHECK-NEXT:bc 12, 20, .LBB0_12
+; CHECK-NEXT:  # %bb.11: # %entry
+; CHECK-NEXT:fmr 6, 8
+; CHECK-NEXT:  .LBB0_12: # %entry
+; CHECK-NEXT:fmr 1, 5
+; CHECK-NEXT:fmr 2, 6
+; CHECK-NEXT:blr
+entry:
+  %0 = call ppc_fp128 (ppc_fp128, ppc_fp128, ppc_fp128, ...) @llvm.ppc.maxfe(ppc_fp128 %a, ppc_fp128 %b, ppc_fp128 %c, ppc_fp128 %d)
+  ret ppc_fp128 %0
+}
+
+declare double @llvm.ppc.maxfl(double %a, double %b, double %c, ...)
+define double @test_maxfl(double %a, double %b, double %c, double %d) {
+; CHECK-P9-LABEL: test_maxfl:
+; CHECK-P9:   # %bb.0: # %entry
+; CHECK-P9-NEXT:xsmaxcdp 0, 3, 2
+; CHECK-P9-NEXT:xsmaxcdp 0, 0, 1
+; CHECK-P9-NEXT:xsmaxcdp 1, 0, 4
+; CHECK-P9-NEXT:blr
+;
+; CHECK-P8-LABEL: test_maxfl:
+; CHECK-P8:   # %bb.0: # %entry
+; CHECK-P8-NEXT:xscmpudp 0, 3, 2
+; CHECK-P8-NEXT:ble 0, .LBB1_4
+; CHECK-P8-NEXT:  # %bb.1: # %entry
+; CHECK-P8-NEXT:xscmpudp 0, 3, 1
+; CHECK-P8-NEXT:ble 0, .LBB1_5
+; CHECK-P8-NEXT:  .LBB1_2: # %entry
+; CHECK-P8-NEXT:xscmpudp 0, 3, 4
+; CHECK-P8-NEXT:ble 0, .LBB1_6
+; CHECK-P8-NEXT:  .LBB1_3: # %entry
+; CHECK-P8-NEXT:fmr 1, 3
+; CHECK-P8-NEXT:blr
+; CHECK-P8-NEXT:  .LBB1_4: # %entry
+; CHECK-P8-NEXT:fmr 3, 2
+; CHECK-P8-NEXT:xscmpudp 0, 3, 1
+; CHECK-P8-NEXT:bgt 0, .LBB1_2
+; CHECK-P8-NEXT:  .LBB1_5: # %entry
+; CHECK-P8-NEXT:fmr 3, 1
+; CHECK-P8-NEXT:xscmpudp 0, 3, 4
+; CHECK-P8-NEXT:bgt 0, .LBB1_3
+; CHECK-P8-NEXT:  .LBB1_6: # %entry
+; CHECK-P8-NEXT:fmr 3, 4
+; CHECK-P8-NEXT:fmr 1, 3
+; CHECK-P8-NEXT:blr
+entry:
+  %0 = call double (double, double, double, ...) @llvm.ppc.maxfl(double %a, double %b, double %c, double %d)
+  ret double %0
+}
+
+declare float @llvm.ppc.maxfs(float %a, float %b, float %c, ...)
+define float @test_maxfs(float %a, float %b, float %c, float %d) {
+; CHECK-P9-LABEL: test_maxfs:
+; CHECK-P9:   # %bb.0: # %entry
+; CHECK-P9-NEXT:xsmaxcdp 0, 3, 2
+; CHECK-P9-NEXT:xsmaxcdp 0, 0, 1
+; CHECK-P9-NEXT:xsmaxcdp 1, 0, 4
+; CHECK-P9-NEXT:blr
+;
+; CHECK-P8-LABEL: test_maxfs:
+; CHECK-P8:   # %bb.0: # %entry
+; CHECK-P8-NEXT:fcmpu 0, 3, 2
+; CHECK-P8-NEXT:ble 0, .LBB2_4
+; CHECK-P8-NEXT:  # %bb.1: # %entry
+; CHECK-P8-NEXT:fcmpu 0, 3, 1
+; CHECK-P8-NEXT:ble 0, .LBB2_5
+; CHECK-P8-NEXT:  .LBB2_2: # %entry
+; CHECK-P8-NEXT:fcmpu 

[PATCH] D122478: [PowerPC] Add max/min intrinsics to Clang and PPC backend

[Ting Wang via Phabricator via cfe-commits]

tingwang added inline comments.



Comment at: llvm/include/llvm/IR/IntrinsicsPowerPC.td:192
+  [llvm_float_ty, llvm_float_ty, llvm_float_ty, 
llvm_vararg_ty],
+  [IntrNoMem]>;
 }

qiucf wrote:
> Will we support `llvm_f128_ty`?
I'm afraid not at this moment. Document mentions only three types: float, 
double, or long double.



Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:10598-10602
+for (--I; Cnt != 0; --Cnt, I = (--I == 0 ? (Op.getNumOperands() - 1) : I)) 
{
+  Res = LowerSELECT_CC(
+  DAG.getSelectCC(dl, Res, Op.getOperand(I), Res, Op.getOperand(I), 
CC),
+  DAG);
+}

qiucf wrote:
> I don't think we need to manually call `LowerSELECT_CC` here. SelectionDAG 
> knows `ppc_fp128` should not be custom lowered.
> 
> This also makes the case pass. Thus D122462 is not needed.
Thank you for pointing out this. Verified LowerSELECT_CC is not required here. 
This was added due to misconception.



Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:11266
+case Intrinsic::ppc_maxfe:
+case Intrinsic::ppc_minfe:
 case Intrinsic::ppc_fnmsub:

qiucf wrote:
> Why only two `fe`?
This is only for ppcf128 during type legalization: 
DAGTypeLegalizer::ExpandFloatResult -> CustomLowerNode ->  
PPCTargetLowering::ReplaceNodeResults. The other cases seem not hitting here. I 
will double check the code path to verify.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122478

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

[PATCH] D122409: [libclang] Add missing CursorKind enums defined in Index.h.

[Tao He via Phabricator via cfe-commits]

sighingnow added a comment.

/cc @arphaman @sstefan1


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122409

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

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

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel marked 2 inline comments as done.
ymandel added inline comments.



Comment at: 
clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp:270
+  // needed.
+  BoolValue &ComparisonValue = MakeValue(Env, *HasValueVal);
+  auto *ComparisonExprLoc =

sgatev wrote:
> ymandel wrote:
> > ymandel wrote:
> > > xazax.hun wrote:
> > > > xazax.hun wrote:
> > > > > ymandel wrote:
> > > > > > xazax.hun wrote:
> > > > > > > Is this the right way to initialize `ComparisonValue`?
> > > > > > > 
> > > > > > > Considering the expression: `opt.value_or(nullptr) != nullptr`
> > > > > > > * When `has_value == false`, `opt.value_or(nullptr)` will return 
> > > > > > > `nullptr`, so `!=` evaluates to false. This case seems to check 
> > > > > > > out.
> > > > > > > * However, when `has_value == true`, `opt` might still hold an 
> > > > > > > `nullptr` and `!=` could still evaluate to false. 
> > > > > > Thanks for digging into this. I think it's correct, but helpful to 
> > > > > > step through:
> > > > > > 
> > > > > > Its correctness depends on `MakeValue`, so I'll focus on that in 
> > > > > > particular. For the `nullptr` case, we'll get:
> > > > > > ```
> > > > > > HasValueVal && ContentsNotEqX
> > > > > > ```
> > > > > > So, when `has_value == true`, this basically reduces to 
> > > > > > `ContentsNotEqX`. Since that's an atom, the result is 
> > > > > > indeterminate, which I believe is the desired outcome.
> > > > > > 
> > > > > > WDYT?  Also, even if I've convinced you, please let me know how i 
> > > > > > can improve the comments. For that matter, would `MakeValue` be 
> > > > > > better with a more specific name, like "MakePredicate" or somesuch?
> > > > > I think what confuses me is that we do something different for the 3 
> > > > > cases. You convinced me that `HasValueVal && ContentsNotEqX` is 
> > > > > correct. But we only do this for one branch out of the 3.  What is 
> > > > > the reason for that?
> > > > Oh, never mind. Yeah, I think changing `MakeValue` to `MakePredicate` 
> > > > would make this a bit clearer. After a second read now I understand 
> > > > better what is going on.
> > > Just to be clear: the three cases you mean are lines 273-283, or 
> > > something else?
> > and never mind my question, then (I rpelied before I saw your updated). 
> > I'll change the name and add comments.
> Can you elaborate on the three cases on lines 273-283? Why not simply do
> 
> ```
> auto &ComparisonExprLoc = Env.createStorageLocation(*ComparisonExpr);
> Env.setStorageLocation(ComparisonExpr, ComparisonExprLoc);
> Env.setValue(ComparisonExprLoc, ComparisonValue);
> ```
> Can you elaborate on the three cases on lines 273-283? Why not simply do
> 
> ```
> auto &ComparisonExprLoc = Env.createStorageLocation(*ComparisonExpr);
> Env.setStorageLocation(ComparisonExpr, ComparisonExprLoc);
> Env.setValue(ComparisonExprLoc, ComparisonValue);
> ```

for the second case: I think we should drop it -- I don't see a reason to 
maintain the previous value (if there is any). It might be a good idea for 
compositionality, but we're not doing that anywhere else, so it doesn't make 
sense here.

for the first and third case: I assumed that if the expression already has a 
location, we'd want to reuse it. But, based on your question, I take it that's 
incorrect?




Comment at: 
clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp:279
+ cast_or_null(Env.getValue(*ComparisonExprLoc))) {
+Env.setValue(*ComparisonExprLoc,
+ Env.makeAnd(*CurrentValue, ComparisonValue));

ymandel wrote:
> xazax.hun wrote:
> > I am still wondering a bit about this case. 
> > 
> > We generate: `HasValueVal and ContentsNotEqX and CurrentValue`.'
> > I wonder if we want: `HasValueVal  and (ContentsNotEqX  <=> CurrentValue)` 
> > instead?  Or even `HasValueVal  and CurrentValue`?
> I don't think that the iff version is right, but `HasValueVal  and 
> CurrentValue` could be. My concern is that we're not guaranteed that 
> `CurrentValue` is populated. And, even if we were, it doesn't feel quite 
> right. Assuming its a high fidelity model, we get (logically): `HasValue(opt) 
> and Ne(ValueOr(opt,X),X)`. Then, when negated (say, on an else branch) we get 
> `not(HasValue(opt)) or not(Ne(ValueOr(opt,X),X))` which is equivalent to 
> `not(HasValue(opt)) or Eq(ValueOr(opt,X),X)`. While  true, it seems 
> redundant, since the first clause should be derivable from the second 
> (assuming an interpretatable semantics to the `ValueOr` predicate).
> 
> Regardless, it might be better to step back and figure out how this should be 
> done systematically. I'll try to come back with a proposal on that.
> Regardless, it might be better to step back and figure out how this should be 
> done systematically. I'll try to come back with a proposal on that.

Here's what I have: in general, we're aiming for all models to be a sound 
(over) app

[PATCH] D122544: Utilize comparison operation implemented in APInt

[Aaron Ballman via Phabricator via cfe-commits]

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

LGTM! Can you also add a release note for the fix?

I'm happy to land this on your behalf if you'd prefer, but given that you've 
had several quality commits already, you might want to consider asking for your 
commit privileges: 
https://llvm.org/docs/DeveloperPolicy.html#obtaining-commit-access.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122544

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

[PATCH] D122556: [RISCV] Add definitions for Xiangshan processors.

[Jessica Clarke via Phabricator via cfe-commits]

jrtc27 added inline comments.



Comment at: llvm/include/llvm/Support/RISCVTargetParser.def:34
 PROC(SIFIVE_U74, {"sifive-u74"}, FK_64BIT, {"rv64gc"})
+PROC(XIANGSHAN_YANQIHU,{"xiangshan-yanqihu"},FK_64BIT,{"rv64gc"})
+PROC(XIANGSHAN_NANHU,{"xiangshan-nanhu"},FK_64BIT,{"rv64imafdc_zba_zbb_zbc_zbs_zbkb_zbkc_zbkx_zknd_zkne_zknh_zksed_zksh"})

Formatting



Comment at: llvm/include/llvm/Support/RISCVTargetParser.def:35
+PROC(XIANGSHAN_YANQIHU,{"xiangshan-yanqihu"},FK_64BIT,{"rv64gc"})
+PROC(XIANGSHAN_NANHU,{"xiangshan-nanhu"},FK_64BIT,{"rv64imafdc_zba_zbb_zbc_zbs_zbkb_zbkc_zbkx_zknd_zkne_zknh_zksed_zksh"})
 

Why imafd rather than g?



Comment at: llvm/lib/Target/RISCV/RISCV.td:546
+
+def : ProcessorModel<"xiangshan-nanhu", NoSchedModel, [Feature64Bit,
+   FeatureStdExtM,

Isn't this still under development?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122556

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

[PATCH] D121637: [PowerPC] Fix EmitPPCBuiltinExpr to emit arguments once

[Amy Kwan via Phabricator via cfe-commits]

amyk added a comment.

Overall this change looks good to me, although I do have one question.




Comment at: clang/lib/CodeGen/CGBuiltin.cpp:15208
-  for (unsigned i = 0, e = E->getNumArgs(); i != e; i++) {
-if (E->getArg(i)->getType()->isArrayType())
-  Ops.push_back(EmitArrayToPointerDecay(E->getArg(i)).getPointer());

A question I have is do we not need to consider 
this/`EmitArrayToPointerDecay()` anymore? Was this not used for anything?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121637

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

[clang] 281b7ee - Update www_status/add test for P1972:

[Erich Keane via cfe-commits]

Author: Erich Keane
Date: 2022-03-28T06:31:04-07:00
New Revision: 281b7eeb149d236dd4f2c1ab976cce361de4e057

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

LOG: Update www_status/add test for P1972:

This seems to have been implemented/supported correctly back in Clang
10, so update the documentation and add the test from the paper.

Added: 
clang/test/SemaCXX/cxx20-check-fptr-constraints.cpp

Modified: 
clang/www/cxx_status.html

Removed: 




diff  --git a/clang/test/SemaCXX/cxx20-check-fptr-constraints.cpp 
b/clang/test/SemaCXX/cxx20-check-fptr-constraints.cpp
new file mode 100644
index 0..3b93e6fc87858
--- /dev/null
+++ b/clang/test/SemaCXX/cxx20-check-fptr-constraints.cpp
@@ -0,0 +1,12 @@
+// RUN: %clang_cc1 -std=c++20 -verify %s
+
+namespace P1972 {
+void f(int) requires false; // expected-note 4{{because 'false' evaluated to 
false}} \
+// expected-note{{constraints not satisfied}}
+void g() {
+  f(0);  // expected-error{{no matching function for call 
to 'f'}}
+  void (*p1)(int) = f;   // expected-error{{invalid reference to function 
'f': constraints not satisfied}}
+  void (*p21)(int) = &f; // expected-error{{invalid reference to function 
'f': constraints not satisfied}}
+  decltype(f) *p2 = nullptr; // expected-error{{invalid reference to function 
'f': constraints not satisfied}}
+}
+}

diff  --git a/clang/www/cxx_status.html b/clang/www/cxx_status.html
index bd863d0f2fc50..3fcf7a3a27cae 100755
--- a/clang/www/cxx_status.html
+++ b/clang/www/cxx_status.html
@@ -938,7 +938,7 @@ C++20 implementation status
   

 https://wg21.link/p1972r0";>P1972R0
-No
+Clang 10
   
   
 https://wg21.link/p1980r0";>P1980R0



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

[PATCH] D122519: [AIX] XFAIL test for lack of visibility support

[Chris Bowler via Phabricator via cfe-commits]

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

The AIX default of ignoring source-level visibility is problematic.  I agree 
the test should be xfailed for AIX.

You can put [NFC] and [tests] to the commit.  Should be safe to commit.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122519

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

[clang] f5a9b5c - [NFC][tests][AIX] XFAIL test for lack of visibility support

[Jake Egan via cfe-commits]

Author: Jake Egan
Date: 2022-03-28T09:43:48-04:00
New Revision: f5a9b5cc12658f4d6caa3e0cfc3e771698fb3798

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

LOG: [NFC][tests][AIX] XFAIL test for lack of visibility support

With the addition of `__attribute__((visibility("hidden")))` to the test, the 
test fails because AIX's current default behaviour is to ignore hidden 
visibility, so the expected error is not seen. This patch marks the test 
`XFAIL` on AIX for now.

Reviewed By: cebowleratibm

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

Added: 


Modified: 
clang/test/OpenMP/target_update_messages.cpp

Removed: 




diff  --git a/clang/test/OpenMP/target_update_messages.cpp 
b/clang/test/OpenMP/target_update_messages.cpp
index f936a075e1b48..d79b579430657 100644
--- a/clang/test/OpenMP/target_update_messages.cpp
+++ b/clang/test/OpenMP/target_update_messages.cpp
@@ -1,3 +1,6 @@
+// Visibility hidden is not currently implemented on AIX.
+// XFAIL: aix
+
 // RUN: %clang_cc1 -verify=expected,lt50,lt51 -fopenmp -fopenmp-version=45 
-ferror-limit 100 -o - -std=c++11 %s -Wuninitialized
 // RUN: %clang_cc1 -verify=expected,ge50,lt51 -fopenmp -fopenmp-version=50 
-ferror-limit 100 -o - -std=c++11 %s -Wuninitialized
 // RUN: %clang_cc1 -verify=expected,ge50,ge51 -fopenmp -fopenmp-version=51 
-ferror-limit 100 -o - -std=c++11 %s -Wuninitialized



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

[PATCH] D122519: [NFC][tests][AIX] XFAIL test for lack of visibility support

[Jake Egan via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGf5a9b5cc1265: [NFC][tests][AIX] XFAIL test for lack of 
visibility support (authored by Jake-Egan).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122519

Files:
  clang/test/OpenMP/target_update_messages.cpp


Index: clang/test/OpenMP/target_update_messages.cpp
===
--- clang/test/OpenMP/target_update_messages.cpp
+++ clang/test/OpenMP/target_update_messages.cpp
@@ -1,3 +1,6 @@
+// Visibility hidden is not currently implemented on AIX.
+// XFAIL: aix
+
 // RUN: %clang_cc1 -verify=expected,lt50,lt51 -fopenmp -fopenmp-version=45 
-ferror-limit 100 -o - -std=c++11 %s -Wuninitialized
 // RUN: %clang_cc1 -verify=expected,ge50,lt51 -fopenmp -fopenmp-version=50 
-ferror-limit 100 -o - -std=c++11 %s -Wuninitialized
 // RUN: %clang_cc1 -verify=expected,ge50,ge51 -fopenmp -fopenmp-version=51 
-ferror-limit 100 -o - -std=c++11 %s -Wuninitialized


Index: clang/test/OpenMP/target_update_messages.cpp
===
--- clang/test/OpenMP/target_update_messages.cpp
+++ clang/test/OpenMP/target_update_messages.cpp
@@ -1,3 +1,6 @@
+// Visibility hidden is not currently implemented on AIX.
+// XFAIL: aix
+
 // RUN: %clang_cc1 -verify=expected,lt50,lt51 -fopenmp -fopenmp-version=45 -ferror-limit 100 -o - -std=c++11 %s -Wuninitialized
 // RUN: %clang_cc1 -verify=expected,ge50,lt51 -fopenmp -fopenmp-version=50 -ferror-limit 100 -o - -std=c++11 %s -Wuninitialized
 // RUN: %clang_cc1 -verify=expected,ge50,ge51 -fopenmp -fopenmp-version=51 -ferror-limit 100 -o - -std=c++11 %s -Wuninitialized
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] 4df69c1 - [ARM] Make testcase warning pattern match more specific

[Ranjeet Singh via cfe-commits]

Author: Ranjeet Singh
Date: 2022-03-28T14:44:15+01:00
New Revision: 4df69c1ff19f75e302cfc3e022009fe971fc5c24

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

LOG: [ARM] Make testcase warning pattern match more specific

Make the warning more specific as downstream compilers could produce other 
warnings.

Reviewed By: tstellar

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

Added: 


Modified: 
clang/test/Driver/aarch64-security-options.c

Removed: 




diff  --git a/clang/test/Driver/aarch64-security-options.c 
b/clang/test/Driver/aarch64-security-options.c
index 4bd73e06d255a..b4bb57e71aa79 100644
--- a/clang/test/Driver/aarch64-security-options.c
+++ b/clang/test/Driver/aarch64-security-options.c
@@ -52,4 +52,4 @@
 
 // Check that the linker driver doesn't warn about -mbranch-protection=standard
 // as an unused option.
-// LINKER-DRIVER-NOT: warning:
+// LINKER-DRIVER-NOT: warning: argument unused during compilation: 
'-mbranch-protection=standard'



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

[PATCH] D122487: [ARM] Make testcase warning pattern match more specific

[Ranjeet Singh via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG4df69c1ff19f: [ARM] Make testcase warning pattern match more 
specific (authored by rs).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122487

Files:
  clang/test/Driver/aarch64-security-options.c


Index: clang/test/Driver/aarch64-security-options.c
===
--- clang/test/Driver/aarch64-security-options.c
+++ clang/test/Driver/aarch64-security-options.c
@@ -52,4 +52,4 @@
 
 // Check that the linker driver doesn't warn about -mbranch-protection=standard
 // as an unused option.
-// LINKER-DRIVER-NOT: warning:
+// LINKER-DRIVER-NOT: warning: argument unused during compilation: 
'-mbranch-protection=standard'


Index: clang/test/Driver/aarch64-security-options.c
===
--- clang/test/Driver/aarch64-security-options.c
+++ clang/test/Driver/aarch64-security-options.c
@@ -52,4 +52,4 @@
 
 // Check that the linker driver doesn't warn about -mbranch-protection=standard
 // as an unused option.
-// LINKER-DRIVER-NOT: warning:
+// LINKER-DRIVER-NOT: warning: argument unused during compilation: '-mbranch-protection=standard'
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D122585: [OpenMP] Handle function declaration or definition omp pragmas in C mode

[Raúl Peñacoba via Phabricator via cfe-commits]

rpenacob created this revision.
rpenacob added reviewers: ABataev, jdoerfert.
Herald added subscribers: guansong, yaxunl.
Herald added a project: All.
rpenacob requested review of this revision.
Herald added subscribers: cfe-commits, sstefan1.
Herald added a project: clang.

Clang crashes when a C++ like struct with a `declare variant` or `declare simd` 
pragma is parsed in C mode. Skip the pragma in this situation and leave the 
parser emit a diagnostic.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D122585

Files:
  clang/lib/Parse/ParseOpenMP.cpp
  clang/test/OpenMP/declare_simd_messages.c
  clang/test/OpenMP/declare_variant_messages.c


Index: clang/test/OpenMP/declare_variant_messages.c
===
--- clang/test/OpenMP/declare_variant_messages.c
+++ clang/test/OpenMP/declare_variant_messages.c
@@ -212,3 +212,16 @@
 #pragma omp declare variant(foo) match(user = {condition(1)}) // 
expected-error {{nested user conditions in OpenMP context selector not 
supported (yet)}}
 int conflicting_nested_condition(void);
 #pragma omp end declare variant
+
+struct S0 {
+  // expected-error@+2 {{field 'foo' declared as a function}}
+  #pragma omp declare variant
+  void foo();
+};
+
+struct S1 {
+  // expected-error@+3 {{field 'bar' declared as a function}}
+  // expected-error@+2 {{expected ';' at end of declaration list}}
+  #pragma omp declare variant
+  void bar() {}
+};
Index: clang/test/OpenMP/declare_simd_messages.c
===
--- /dev/null
+++ clang/test/OpenMP/declare_simd_messages.c
@@ -0,0 +1,14 @@
+// RUN: %clang_cc1 -triple=x86_64-pc-win32 -verify -fopenmp -x c 
-fms-extensions -Wno-pragma-pack %s
+
+struct S0 {
+  // expected-error@+2 {{field 'foo' declared as a function}}
+  #pragma omp declare simd
+  void foo();
+};
+
+struct S1 {
+  // expected-error@+3 {{field 'bar' declared as a function}}
+  // expected-error@+2 {{expected ';' at end of declaration list}}
+  #pragma omp declare simd
+  void bar() {}
+};
Index: clang/lib/Parse/ParseOpenMP.cpp
===
--- clang/lib/Parse/ParseOpenMP.cpp
+++ clang/lib/Parse/ParseOpenMP.cpp
@@ -2262,6 +2262,9 @@
   }
   case OMPD_declare_variant:
   case OMPD_declare_simd: {
+// Only allowed in C++ mode
+if (!getLangOpts().CPlusPlus && getCurScope()->isClassScope())
+  break;
 // The syntax is:
 // { #pragma omp declare {simd|variant} }
 // 


Index: clang/test/OpenMP/declare_variant_messages.c
===
--- clang/test/OpenMP/declare_variant_messages.c
+++ clang/test/OpenMP/declare_variant_messages.c
@@ -212,3 +212,16 @@
 #pragma omp declare variant(foo) match(user = {condition(1)}) // expected-error {{nested user conditions in OpenMP context selector not supported (yet)}}
 int conflicting_nested_condition(void);
 #pragma omp end declare variant
+
+struct S0 {
+  // expected-error@+2 {{field 'foo' declared as a function}}
+  #pragma omp declare variant
+  void foo();
+};
+
+struct S1 {
+  // expected-error@+3 {{field 'bar' declared as a function}}
+  // expected-error@+2 {{expected ';' at end of declaration list}}
+  #pragma omp declare variant
+  void bar() {}
+};
Index: clang/test/OpenMP/declare_simd_messages.c
===
--- /dev/null
+++ clang/test/OpenMP/declare_simd_messages.c
@@ -0,0 +1,14 @@
+// RUN: %clang_cc1 -triple=x86_64-pc-win32 -verify -fopenmp -x c -fms-extensions -Wno-pragma-pack %s
+
+struct S0 {
+  // expected-error@+2 {{field 'foo' declared as a function}}
+  #pragma omp declare simd
+  void foo();
+};
+
+struct S1 {
+  // expected-error@+3 {{field 'bar' declared as a function}}
+  // expected-error@+2 {{expected ';' at end of declaration list}}
+  #pragma omp declare simd
+  void bar() {}
+};
Index: clang/lib/Parse/ParseOpenMP.cpp
===
--- clang/lib/Parse/ParseOpenMP.cpp
+++ clang/lib/Parse/ParseOpenMP.cpp
@@ -2262,6 +2262,9 @@
   }
   case OMPD_declare_variant:
   case OMPD_declare_simd: {
+// Only allowed in C++ mode
+if (!getLangOpts().CPlusPlus && getCurScope()->isClassScope())
+  break;
 // The syntax is:
 // { #pragma omp declare {simd|variant} }
 // 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D122378: [doc] Rely on tblgen to dump supported options value when generating doc

[serge via Phabricator via cfe-commits]

serge-sans-paille updated this revision to Diff 418579.
serge-sans-paille retitled this revision from "Be more explicit about 
-fvisibility= documentation" to "[doc] Rely on tblgen to dump supported 
options value when generating doc".
serge-sans-paille edited the summary of this revision.
serge-sans-paille added a comment.
Herald added subscribers: llvm-commits, aheejin.
Herald added a project: LLVM.

Moved to a generic approach as suggested by reviewer.


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

https://reviews.llvm.org/D122378

Files:
  clang/include/clang/Driver/Options.td
  llvm/utils/TableGen/OptRSTEmitter.cpp

Index: llvm/utils/TableGen/OptRSTEmitter.cpp
===
--- llvm/utils/TableGen/OptRSTEmitter.cpp
+++ llvm/utils/TableGen/OptRSTEmitter.cpp
@@ -60,18 +60,45 @@
   // Print the option name.
   OS << R->getValueAsString("Name");
 
+  StringRef MetaVarName;
   // Print the meta-variable.
   if (!isa(R->getValueInit("MetaVarName"))) {
+MetaVarName = R->getValueAsString("MetaVarName");
+  } else if (!isa(R->getValueInit("Values")))
+MetaVarName = "";
+
+  if (!MetaVarName.empty()) {
 OS << '=';
-OS.write_escaped(R->getValueAsString("MetaVarName"));
+OS.write_escaped(MetaVarName);
   }
 
   OS << "\n\n";
 
+  std::string HelpText;
   // The option help text.
   if (!isa(R->getValueInit("HelpText"))) {
+HelpText = R->getValueAsString("HelpText").trim().str();
+if (!HelpText.empty() && HelpText.back() != '.')
+  HelpText.push_back('.');
+  }
+
+  if (!isa(R->getValueInit("Values"))) {
+SmallVector Values;
+SplitString(R->getValueAsString("Values"), Values, ",");
+HelpText += (" " + MetaVarName + " can be ").str();
+
+if (Values.size() == 1) {
+  HelpText += ("'" + Values.front() + "'.").str();
+} else {
+  HelpText += "one of '";
+  HelpText += join(Values.begin(), Values.end() - 1, "', '");
+  HelpText += ("' or '" + Values.back() + "'.").str();
+}
+  }
+
+  if (!HelpText.empty()) {
 OS << ' ';
-OS.write_escaped(R->getValueAsString("HelpText"));
+OS.write_escaped(HelpText);
 OS << "\n\n";
   }
 }
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -960,7 +960,7 @@
   PosFlag,
   NegFlag>;
 def fgpu_default_stream_EQ : Joined<["-"], "fgpu-default-stream=">,
-  HelpText<"Specify default stream. Valid values are 'legacy' and 'per-thread'. The default value is 'legacy'. (HIP only)">,
+  HelpText<"Specify default stream. The default value is 'legacy'. (HIP only)">,
   Flags<[CC1Option]>,
   Values<"legacy,per-thread">,
   NormalizedValuesScope<"LangOptions::GPUDefaultStreamKind">,
@@ -1171,7 +1171,7 @@
   MarshallingInfoStringVector>;
 def fembed_bitcode_EQ : Joined<["-"], "fembed-bitcode=">,
 Group, Flags<[NoXarchOption, CC1Option, CC1AsOption]>, MetaVarName<"">,
-HelpText<"Embed LLVM bitcode (option: off, all, bitcode, marker)">,
+HelpText<"Embed LLVM bitcode">,
 Values<"off,all,bitcode,marker">, NormalizedValuesScope<"CodeGenOptions">,
 NormalizedValues<["Embed_Off", "Embed_All", "Embed_Bitcode", "Embed_Marker"]>,
 MarshallingInfoEnum, "Embed_Off">;
@@ -1297,7 +1297,7 @@
 ShouldParseIf;
 def fprofile_update_EQ : Joined<["-"], "fprofile-update=">,
 Group, Flags<[CC1Option, CoreOption]>, Values<"atomic,prefer-atomic,single">,
-MetaVarName<"">, HelpText<"Set update method of profile counters (atomic,prefer-atomic,single)">,
+MetaVarName<"">, HelpText<"Set update method of profile counters">,
 MarshallingInfoFlag>;
 defm pseudo_probe_for_profiling : BoolFOption<"pseudo-probe-for-profiling",
   CodeGenOpts<"PseudoProbeForProfiling">, DefaultFalse,
@@ -1312,7 +1312,7 @@
 MarshallingInfoStringVector>;
 def fswift_async_fp_EQ : Joined<["-"], "fswift-async-fp=">,
 Group, Flags<[CC1Option, CC1AsOption, CoreOption]>, MetaVarName<"">,
-HelpText<"Control emission of Swift async extended frame info (option: auto, always, never)">,
+HelpText<"Control emission of Swift async extended frame info">,
 Values<"auto,always,never">,
 NormalizedValuesScope<"CodeGenOptions::SwiftAsyncFramePointerKind">,
 NormalizedValues<["Auto", "Always", "Never"]>,
@@ -1483,7 +1483,7 @@
 def fwasm_exceptions : Flag<["-"], "fwasm-exceptions">, Group,
   HelpText<"Use WebAssembly style exceptions">;
 def exception_model : Separate<["-"], "exception-model">,
-  Flags<[CC1Option, NoDriverOption]>, HelpText<"The exception model: dwarf|sjlj|seh|wasm">,
+  Flags<[CC1Option, NoDriverOption]>, HelpText<"The exception model">,
   Values<"dwarf,sjlj,seh,wasm">,
   NormalizedValuesScope<"LangOptions::ExceptionHa

[PATCH] D122008: [flang][driver] Add support for generating executables

[Andrzej Warzynski via Phabricator via cfe-commits]

awarzynski updated this revision to Diff 418580.
awarzynski added a comment.
Herald added a subscriber: MaskRay.

Use llvm_add_library instead of add_flang_library

Fortran_main should be a static library, but `add_flang_library` does not parse
`STATIC` in `add_flang_library(Fortran_main STATIC Fortran_main.c)`. I could
extend `add_flang_library` instead, but `llvm_add_library` works fine as well
and this change is less intrusive.

Also rebased on top of main.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122008

Files:
  clang/lib/Driver/ToolChains/Gnu.cpp
  flang/include/flang/Runtime/stop.h
  flang/runtime/CMakeLists.txt
  flang/runtime/FortranMain/CMakeLists.txt
  flang/runtime/FortranMain/Fortran_main.c
  flang/test/CMakeLists.txt
  flang/test/Driver/linker-flags.f90

Index: flang/test/Driver/linker-flags.f90
===
--- /dev/null
+++ flang/test/Driver/linker-flags.f90
@@ -0,0 +1,30 @@
+! Verify that the Fortran runtime libraries are present in the linker
+! invocation. These libraries are added on top of other standard runtime
+! libraries that the Clang driver will include.
+
+! NOTE: The additional linker flags tested here are currently specified in
+! clang/lib/Driver/Toolchains/Gnu.cpp. This makes the current implementation GNU
+! (Linux) specific. The following line will make sure that this test is skipped
+! on Windows. Ideally we should find a more robust way of testing this.
+! REQUIRES: shell
+! UNSUPPORTED: darwin, macos
+
+!
+! RUN COMMAND
+!
+! RUN: %flang -### --ld-path=/usr/bin/ld %S/Inputs/hello.f90 2>&1 | FileCheck %s
+
+!
+! EXPECTED OUTPUT
+!
+! Compiler invocation to generate the object file
+! CHECK-LABEL: {{.*}} "-emit-obj"
+! CHECK-SAME:  "-o" "[[object_file:.*]]" {{.*}}Inputs/hello.f90
+
+! Linker invocation to generate the executable
+! CHECK-LABEL:  "/usr/bin/ld"
+! CHECK-SAME: "[[object_file]]"
+! CHECK-SAME: -lFortran_main
+! CHECK-SAME: -lFortranRuntime
+! CHECK-SAME: -lFortranDecimal
+! CHECK-SAME: -lm
Index: flang/test/CMakeLists.txt
===
--- flang/test/CMakeLists.txt
+++ flang/test/CMakeLists.txt
@@ -57,6 +57,9 @@
   bbc
   llvm-objdump
   split-file
+  FortranRuntime
+  Fortran_main
+  FortranDecimal
 )
 
 if (FLANG_INCLUDE_TESTS)
Index: flang/runtime/FortranMain/Fortran_main.c
===
--- /dev/null
+++ flang/runtime/FortranMain/Fortran_main.c
@@ -0,0 +1,21 @@
+//===-- runtime/FortranMain/Fortran_main.c ===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "flang/Runtime/main.h"
+#include "flang/Runtime/stop.h"
+
+/* main entry into PROGRAM */
+void _QQmain();
+
+/* C main stub */
+int main(int argc, const char *argv[], const char *envp[]) {
+  RTNAME(ProgramStart)(argc, argv, envp);
+  _QQmain();
+  RTNAME(ProgramEndStatement)();
+  return 0;
+}
Index: flang/runtime/FortranMain/CMakeLists.txt
===
--- /dev/null
+++ flang/runtime/FortranMain/CMakeLists.txt
@@ -0,0 +1,3 @@
+llvm_add_library(Fortran_main STATIC
+  Fortran_main.c
+)
Index: flang/runtime/CMakeLists.txt
===
--- flang/runtime/CMakeLists.txt
+++ flang/runtime/CMakeLists.txt
@@ -30,6 +30,8 @@
 # with different names
 include_directories(AFTER ${CMAKE_CURRENT_BINARY_DIR})
 
+add_subdirectory(FortranMain)
+
 add_flang_library(FortranRuntime
   ISO_Fortran_binding.cpp
   allocatable.cpp
Index: flang/include/flang/Runtime/stop.h
===
--- flang/include/flang/Runtime/stop.h
+++ flang/include/flang/Runtime/stop.h
@@ -27,7 +27,7 @@
 NORETURN void RTNAME(ProgramEndStatement)(NO_ARGUMENTS);
 
 // Extensions
-NORETURN void RTNAME(Exit)(int status = EXIT_SUCCESS);
+NORETURN void RTNAME(Exit)(int status DEFAULT_VALUE(EXIT_SUCCESS));
 NORETURN void RTNAME(Abort)(NO_ARGUMENTS);
 
 // Crash with an error message when the program dynamically violates a Fortran
Index: clang/lib/Driver/ToolChains/Gnu.cpp
===
--- clang/lib/Driver/ToolChains/Gnu.cpp
+++ clang/lib/Driver/ToolChains/Gnu.cpp
@@ -379,6 +379,13 @@
  Exec, CmdArgs, Inputs, Output));
 }
 
+static void addFortranLinkerFlags(ArgStringList &CmdArgs) {
+  CmdArgs.push_back("-lFortran_main");
+  CmdArgs.push_back("-lFortranRuntime");
+  CmdArgs.push_back("-lFortranDecimal");
+  CmdA

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

[Sam Elliott via Phabricator via cfe-commits]

lenary added a comment.

In D119720#3339855 , @dmgreen wrote:

> I have a high level question regarding RDF, as I've not seen it used in many 
> other places, so it may be under-tested on Arm systems at the moment. This 
> currently, for all code, builds an rdf graph, analyze the rdf graph for a 
> fairly rare instructions, then fixes up the code based on that.  It might be 
> best to avoid the (possibly expensive?) rdf graph generation for the common 
> case where the instructions are not present. Check that the instruction 
> exists first.

I haven't had time to see what effect this has on compile times, but I don't 
see the point in iterating over every instruction in the function checking the 
opcode, to then iterate over them all constructing the rdfgraph, that seems to 
make the approach a lot more expensive, if I'm then going to iterate over the 
rdfgraph looking for specific instructions again.

> It might then be simpler to just search back for the def of a register, 
> considering in most code the instruction we are looking for should be fairly 
> rare and we won't expect to need to find def's in bulk. That might be simpler 
> overall, and avoid some of the difficulties of RDF.

The reason for using the rdfgraph was to allow us to improve this at a later 
date if we found the performance issues a problem, i.e. by hoisting the `vorr` 
past phis so we can save on executing them in loops. Given the issues with 
rdfgraph, I'm going to reimplement this as a Block-local analysis and fixup 
pass without the rdfgraph, which is effectively what the pass does in the 
current patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119720

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

[PATCH] D122553: [Driver][AVR] Fix warn_drv_avr_stdlib_not_linked condition

[Ben Shi via Phabricator via cfe-commits]

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



Comment at: clang/lib/Driver/ToolChains/AVR.cpp:451
+  !Args.hasArg(options::OPT_nodefaultlibs)) {
+if (CPU.empty()) {
+  // We cannot link any standard libraries without an MCU specified.

I think we should warn empty CPU name in the compile stage. For example, if 
user specified `-c` but not `-mmcu`, then we can not run to here to warn cpu is 
empty.

So I will accept and rebase on your code.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122553

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

[PATCH] D122586: Fix template instantiation of UDLs

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman created this revision.
aaron.ballman added reviewers: erichkeane, rsmith, clang-language-wg.
Herald added a project: All.
aaron.ballman requested review of this revision.
Herald added a project: clang.

Previously, we would instantiate the UDL by marking the function as referenced 
and potentially binding to a temporary; this skipped transforming the call when 
the UDL was dependent on a template parameter.

Now, we defer all the work to instantiating the call expression for the UDL. 
This ensures that constant evaluation occurs at compile time rather than 
deferring until runtime.

Fixes Issue 54578.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D122586

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/Sema/TreeTransform.h
  clang/test/CodeGenCXX/cxx20-consteval-crash.cpp
  clang/test/SemaCXX/cxx2a-consteval.cpp


Index: clang/test/SemaCXX/cxx2a-consteval.cpp
===
--- clang/test/SemaCXX/cxx2a-consteval.cpp
+++ clang/test/SemaCXX/cxx2a-consteval.cpp
@@ -721,3 +721,27 @@
  expected-note {{subobject of type 'int' is not initialized}}
 
 } // namespace NamespaceScopeConsteval
+
+namespace Issue54578 {
+// We expect the user-defined literal to be resovled entirely at compile time
+// despite being instantiated through a template.
+inline consteval unsigned char operator""_UC(const unsigned long long n) {
+  return static_cast(n);
+}
+
+inline constexpr char f1(const auto octet) {
+  return 4_UC;
+}
+
+template 
+inline constexpr char f2(const Ty octet) {
+  return 4_UC;
+}
+
+void test() {
+  static_assert(f1('a') == 4);
+  static_assert(f2('a') == 4);
+  constexpr int c = f1('a') + f2('a');
+  static_assert(c == 8);
+}
+}
Index: clang/test/CodeGenCXX/cxx20-consteval-crash.cpp
===
--- clang/test/CodeGenCXX/cxx20-consteval-crash.cpp
+++ clang/test/CodeGenCXX/cxx20-consteval-crash.cpp
@@ -24,3 +24,29 @@
 // CHECK:  ret void
 // CHECK: }
 }
+
+namespace Issue54578 {
+inline consteval unsigned char operator""_UC(const unsigned long long n) {
+  return static_cast(n);
+}
+
+inline constexpr char f1(const auto octet) {
+  return 4_UC;
+}
+
+template 
+inline constexpr char f2(const Ty octet) {
+  return 4_UC;
+}
+
+int foo() {
+  return f1('a') + f2('a');
+}
+
+// CHECK-NOT: define{{.*}} zeroext i8 @_ZN10Issue54578li3_UCEy
+// CHECK: define{{.*}} i32 @_ZN10Issue545783fooEv(
+// CHECK: define{{.*}} signext i8 @_ZN10Issue545782f1IcEEcT_(
+// CHECK: ret i8 4
+// CHECK: define{{.*}} signext i8 @_ZN10Issue545782f2IcEEcT_(
+// CHECK: ret i8 4
+}
Index: clang/lib/Sema/TreeTransform.h
===
--- clang/lib/Sema/TreeTransform.h
+++ clang/lib/Sema/TreeTransform.h
@@ -10513,9 +10513,7 @@
 template
 ExprResult
 TreeTransform::TransformUserDefinedLiteral(UserDefinedLiteral *E) {
-  if (FunctionDecl *FD = E->getDirectCallee())
-SemaRef.MarkFunctionReferenced(E->getBeginLoc(), FD);
-  return SemaRef.MaybeBindToTemporary(E);
+  return TransformCallExpr(E);
 }
 
 template
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -189,6 +189,9 @@
 ^
 - Diagnose consteval and constexpr issues that happen at namespace scope. This
   partially addresses `Issue 51593 
`_.
+- No longer attempt to evaluate a consteval UDL function call at runtime when
+  it is called through a template instantiation. This fixes
+  `Issue 54578 `_.
 
 C++2b Feature Support
 ^


Index: clang/test/SemaCXX/cxx2a-consteval.cpp
===
--- clang/test/SemaCXX/cxx2a-consteval.cpp
+++ clang/test/SemaCXX/cxx2a-consteval.cpp
@@ -721,3 +721,27 @@
  expected-note {{subobject of type 'int' is not initialized}}
 
 } // namespace NamespaceScopeConsteval
+
+namespace Issue54578 {
+// We expect the user-defined literal to be resovled entirely at compile time
+// despite being instantiated through a template.
+inline consteval unsigned char operator""_UC(const unsigned long long n) {
+  return static_cast(n);
+}
+
+inline constexpr char f1(const auto octet) {
+  return 4_UC;
+}
+
+template 
+inline constexpr char f2(const Ty octet) {
+  return 4_UC;
+}
+
+void test() {
+  static_assert(f1('a') == 4);
+  static_assert(f2('a') == 4);
+  constexpr int c = f1('a') + f2('a');
+  static_assert(c == 8);
+}
+}
Index: clang/test/CodeGenCXX/cxx20-consteval-crash.cpp
===
--- clang/test/CodeGenCXX/cxx20-consteval-crash.cpp
+++ clang/test/CodeGenCXX/cxx20-consteval-crash.cpp
@@ -24,3 +24,29 @@
 // CHECK:  ret void
 // CHECK: }
 }
+
+namespace Issue54578 {
+inline consteval

[PATCH] D122586: Fix template instantiation of UDLs

[Erich Keane via Phabricator via cfe-commits]

erichkeane added inline comments.



Comment at: clang/lib/Sema/TreeTransform.h:10516
 TreeTransform::TransformUserDefinedLiteral(UserDefinedLiteral *E) {
-  if (FunctionDecl *FD = E->getDirectCallee())
-SemaRef.MarkFunctionReferenced(E->getBeginLoc(), FD);
-  return SemaRef.MaybeBindToTemporary(E);
+  return TransformCallExpr(E);
 }

I THINK you have to do this by doing `getDerived().TransformCallExpr(E)`.  



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122586

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

[PATCH] D122586: Fix template instantiation of UDLs

[Corentin Jabot via Phabricator via cfe-commits]

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

LGTM.




Comment at: clang/test/CodeGenCXX/cxx20-consteval-crash.cpp:28
+
+namespace Issue54578 {
+inline consteval unsigned char operator""_UC(const unsigned long long n) {

I've used things like `gh` previously. for my own curiosity, is there a 
convention here?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122586

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

[PATCH] D122586: Fix template instantiation of UDLs

[Corentin Jabot via Phabricator via cfe-commits]

cor3ntin added inline comments.



Comment at: clang/lib/Sema/TreeTransform.h:10516
 TreeTransform::TransformUserDefinedLiteral(UserDefinedLiteral *E) {
-  if (FunctionDecl *FD = E->getDirectCallee())
-SemaRef.MarkFunctionReferenced(E->getBeginLoc(), FD);
-  return SemaRef.MaybeBindToTemporary(E);
+  return TransformCallExpr(E);
 }

erichkeane wrote:
> I THINK you have to do this by doing `getDerived().TransformCallExpr(E)`.  
> 
Oh yes, good point


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122586

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

[PATCH] D122586: Fix template instantiation of UDLs

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/lib/Sema/TreeTransform.h:10516
 TreeTransform::TransformUserDefinedLiteral(UserDefinedLiteral *E) {
-  if (FunctionDecl *FD = E->getDirectCallee())
-SemaRef.MarkFunctionReferenced(E->getBeginLoc(), FD);
-  return SemaRef.MaybeBindToTemporary(E);
+  return TransformCallExpr(E);
 }

cor3ntin wrote:
> erichkeane wrote:
> > I THINK you have to do this by doing `getDerived().TransformCallExpr(E)`.  
> > 
> Oh yes, good point
Hmmm, don't we want this level of transformation to kick in, not the derived? 
e.g., another approach would be to remove the definition of this function 
entirely so that the recursive AST visitor calls `TransformCallExpr()` instead?



Comment at: clang/test/CodeGenCXX/cxx20-consteval-crash.cpp:28
+
+namespace Issue54578 {
+inline consteval unsigned char operator""_UC(const unsigned long long n) {

cor3ntin wrote:
> I've used things like `gh` previously. for my own curiosity, is there a 
> convention here?
I don't know if we've organically gotten a convention here yet or not. I've 
been using `Issue` for mine, but don't have a strong preference either.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122586

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

[PATCH] D122586: Fix template instantiation of UDLs

[Erich Keane via Phabricator via cfe-commits]

erichkeane added inline comments.



Comment at: clang/lib/Sema/TreeTransform.h:10516
 TreeTransform::TransformUserDefinedLiteral(UserDefinedLiteral *E) {
-  if (FunctionDecl *FD = E->getDirectCallee())
-SemaRef.MarkFunctionReferenced(E->getBeginLoc(), FD);
-  return SemaRef.MaybeBindToTemporary(E);
+  return TransformCallExpr(E);
 }

aaron.ballman wrote:
> cor3ntin wrote:
> > erichkeane wrote:
> > > I THINK you have to do this by doing `getDerived().TransformCallExpr(E)`. 
> > >  
> > > 
> > Oh yes, good point
> Hmmm, don't we want this level of transformation to kick in, not the derived? 
> e.g., another approach would be to remove the definition of this function 
> entirely so that the recursive AST visitor calls `TransformCallExpr()` 
> instead?
No, the idea is that `TreeTransform` is inherited by other 'instantiator' 
types.  IF those types do something special for `CallExpr`, we want this to 
ALSO do the same thing as `CallExpr`.  

I was also thinking about removing the function altogether.  I just couldn't 
remember if `TreeTransform` did that right :) I'd suggest that if possible.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122586

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

[PATCH] D122587: [clang][NFC] Extract the EmitAssemblyHelper::TargetTriple member

[Pavel Samolysov via Phabricator via cfe-commits]

psamolysov-intel created this revision.
psamolysov-intel added reviewers: tejohnson, ddunbar, tobiasvk, tobiasvk_caf.
psamolysov-intel added a project: clang.
Herald added a subscriber: ormris.
Herald added a project: All.
psamolysov-intel requested review of this revision.
Herald added a subscriber: cfe-commits.

Few times in different methods of the EmitAssemblyHelper class the following
code snippet is used to get the TargetTriple and then use it's single method
to check some conditions:

TargetTriple(TheModule->getTargetTriple())

The parsing of a target triple string is not a trivial operation and it takes
time to repeat the parsing many times in different methods of the class and
even numerous times in one method just to call a getter
(llvm::Triple(TheModule->getTargetTriple()).getVendor()), for example.
The patch extracts the TargetTriple member of the EmitAssemblyHelper class to
parse the triple only once in the class' constructor.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D122587

Files:
  clang/lib/CodeGen/BackendUtil.cpp


Index: clang/lib/CodeGen/BackendUtil.cpp
===
--- clang/lib/CodeGen/BackendUtil.cpp
+++ clang/lib/CodeGen/BackendUtil.cpp
@@ -118,6 +118,8 @@
 
   std::unique_ptr OS;
 
+  Triple TargetTriple;
+
   TargetIRAnalysis getTargetIRAnalysis() const {
 if (TM)
   return TM->getTargetIRAnalysis();
@@ -170,7 +172,8 @@
  const LangOptions &LOpts, Module *M)
   : Diags(_Diags), HSOpts(HeaderSearchOpts), CodeGenOpts(CGOpts),
 TargetOpts(TOpts), LangOpts(LOpts), TheModule(M),
-CodeGenerationTime("codegen", "Code Generation Time") {}
+CodeGenerationTime("codegen", "Code Generation Time"),
+TargetTriple(TheModule->getTargetTriple()) {}
 
   ~EmitAssemblyHelper() {
 if (CodeGenOpts.DisableFree)
@@ -694,7 +697,6 @@
   // manually (and not via PMBuilder), since some passes (eg. InstrProfiling)
   // are inserted before PMBuilder ones - they'd get the default-constructed
   // TLI with an unknown target otherwise.
-  Triple TargetTriple(TheModule->getTargetTriple());
   std::unique_ptr TLII(
   createTLII(TargetTriple, CodeGenOpts));
 
@@ -964,7 +966,6 @@
raw_pwrite_stream &OS,
raw_pwrite_stream *DwoOS) {
   // Add LibraryInfo.
-  llvm::Triple TargetTriple(TheModule->getTargetTriple());
   std::unique_ptr TLII(
   createTLII(TargetTriple, CodeGenOpts));
   CodeGenPasses.add(new TargetLibraryInfoWrapperPass(*TLII));
@@ -1053,10 +1054,8 @@
   // Emit a module summary by default for Regular LTO except for ld64
   // targets
   bool EmitLTOSummary =
-  (CodeGenOpts.PrepareForLTO &&
-   !CodeGenOpts.DisableLLVMPasses &&
-   llvm::Triple(TheModule->getTargetTriple()).getVendor() !=
-   llvm::Triple::Apple);
+  (CodeGenOpts.PrepareForLTO && !CodeGenOpts.DisableLLVMPasses &&
+   TargetTriple.getVendor() != llvm::Triple::Apple);
   if (EmitLTOSummary) {
 if (!TheModule->getModuleFlag("ThinLTO"))
   TheModule->addModuleFlag(Module::Error, "ThinLTO", uint32_t(0));
@@ -1337,7 +1336,6 @@
 
   // Register the target library analysis directly and give it a customized
   // preset TLI.
-  Triple TargetTriple(TheModule->getTargetTriple());
   std::unique_ptr TLII(
   createTLII(TargetTriple, CodeGenOpts));
   FAM.registerPass([&] { return TargetLibraryAnalysis(*TLII); });
@@ -1473,8 +1471,7 @@
   // targets
   bool EmitLTOSummary =
   (CodeGenOpts.PrepareForLTO && !CodeGenOpts.DisableLLVMPasses &&
-   llvm::Triple(TheModule->getTargetTriple()).getVendor() !=
-   llvm::Triple::Apple);
+   TargetTriple.getVendor() != llvm::Triple::Apple);
   if (EmitLTOSummary) {
 if (!TheModule->getModuleFlag("ThinLTO"))
   TheModule->addModuleFlag(Module::Error, "ThinLTO", uint32_t(0));


Index: clang/lib/CodeGen/BackendUtil.cpp
===
--- clang/lib/CodeGen/BackendUtil.cpp
+++ clang/lib/CodeGen/BackendUtil.cpp
@@ -118,6 +118,8 @@
 
   std::unique_ptr OS;
 
+  Triple TargetTriple;
+
   TargetIRAnalysis getTargetIRAnalysis() const {
 if (TM)
   return TM->getTargetIRAnalysis();
@@ -170,7 +172,8 @@
  const LangOptions &LOpts, Module *M)
   : Diags(_Diags), HSOpts(HeaderSearchOpts), CodeGenOpts(CGOpts),
 TargetOpts(TOpts), LangOpts(LOpts), TheModule(M),
-CodeGenerationTime("codegen", "Code Generation Time") {}
+CodeGenerationTime("codegen", "Code Generation Time"),
+TargetTriple(TheModule->getTargetTriple()) {}
 
   ~EmitAssemblyHelper() {
 if (CodeGenOpts.DisableFree)
@@ -694,7 +697,6 @@
   // manually (and not via PMBuilder), since some passes (eg. InstrProfiling)
   // are inserted before PMBuilder ones - they'd get

[PATCH] D122587: [clang][NFC] Extract the EmitAssemblyHelper::TargetTriple member

[Pavel Samolysov via Phabricator via cfe-commits]

psamolysov-intel added a comment.

I see some code duplication on lines 1056-1065 and 1472-1481, I have a fix for 
this and going to open a new review after this patch will be landed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122587

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

[PATCH] D120989: Support debug info for alias variable

[Kavitha Natarajan via Phabricator via cfe-commits]

kavitha-natarajan added a comment.

> Thanks for the details - can you ping this thread once the gdb thread has 
> progressed/seems like it's moving in this direction?

@dblaikie, received comments for the gdb patch 
https://sourceware.org/pipermail/gdb-patches/2022-March/186960.html and the 
next message with my response. Can you please look through?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120989

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

[PATCH] D122155: Add warning when eval-method is set in the presence of value unsafe floating-point calculations.

[Zahira Ammarguellat via Phabricator via cfe-commits]

zahiraam updated this revision to Diff 418597.
zahiraam marked 5 inline comments as done.

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

https://reviews.llvm.org/D122155

Files:
  clang/include/clang/Basic/DiagnosticFrontendKinds.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/Sema/SemaAttr.cpp
  clang/test/Sema/eval-method-with-unsafe-math.c

Index: clang/test/Sema/eval-method-with-unsafe-math.c
===
--- /dev/null
+++ clang/test/Sema/eval-method-with-unsafe-math.c
@@ -0,0 +1,82 @@
+// RUN: not %clang_cc1 -fexperimental-strict-floating-point \
+// RUN: -triple x86_64-linux-gnu  \
+// RUN: -verify %s 2>&1 | FileCheck %s --check-prefixes=CHECK-PRGM
+
+// RUN: not %clang_cc1 -fexperimental-strict-floating-point \
+// RUN: -triple x86_64-linux-gnu  -freciprocal-math \
+// RUN: -verify %s 2>&1 | FileCheck %s --check-prefixes=CHECK-RECPR,CHECK-PRGM
+
+// RUN: not %clang_cc1 -fexperimental-strict-floating-point \
+// RUN: -triple x86_64-linux-gnu  -mreassociate \
+// RUN: -verify %s 2>&1 | FileCheck %s --check-prefixes=CHECK-ASSOC,CHECK-PRGM
+
+// RUN: not %clang_cc1 -fexperimental-strict-floating-point \
+// RUN: -triple x86_64-linux-gnu  -fapprox-func \
+// RUN: -verify %s 2>&1 | FileCheck %s --check-prefixes=CHECK-FUNC,CHECK-PRGM
+
+// RUN: not %clang_cc1 -fexperimental-strict-floating-point \
+// RUN: -triple x86_64-linux-gnu -freciprocal-math -mreassociate -verify \
+// RUN: %s 2>&1 | FileCheck %s --check-prefixes=CHECK-ASSOC,CHECK-RECPR,CHECK-PRGM
+
+// RUN: not %clang_cc1 -fexperimental-strict-floating-point \
+// RUN: -triple x86_64-linux-gnu -freciprocal-math -mreassociate -fapprox-func \
+// RUN: -verify %s 2>&1 \
+// RUN: | FileCheck %s --check-prefixes=CHECK-FUNC,CHECK-ASSOC,CHECK-RECPR,CHECK-PRGM
+
+// RUN: not %clang_cc1 -fexperimental-strict-floating-point \
+// RUN: -triple x86_64-linux-gnu -fapprox-func -ffp-eval-method=source \
+// RUN: -verify %s 2>&1 | FileCheck %s --check-prefixes=CHECK-FE-FUNC
+
+// RUN: not %clang_cc1 -fexperimental-strict-floating-point \
+// RUN: -triple x86_64-linux-gnu -mreassociate -ffp-eval-method=source \
+// RUN: -verify %s 2>&1 | FileCheck %s --check-prefixes=CHECK-FE-ASSOC
+
+// RUN: not %clang_cc1 -fexperimental-strict-floating-point \
+// RUN: -triple x86_64-linux-gnu -freciprocal-math -ffp-eval-method=source \
+// RUN: -verify %s 2>&1 | FileCheck %s --check-prefixes=CHECK-FE-RECPR
+
+// RUN: not %clang_cc1 -fexperimental-strict-floating-point \
+// RUN: -triple x86_64-linux-gnu -freciprocal-math -mreassociate \
+// RUN: -ffp-eval-method=source \
+// RUN: -verify %s 2>&1 | FileCheck %s --check-prefixes=CHECK-FE-ASSOC,CHECK-FE-RECPR
+
+// RUN: not %clang_cc1 -fexperimental-strict-floating-point \
+// RUN: -triple x86_64-linux-gnu -freciprocal-math -mreassociate -fapprox-func \
+// RUN: -ffp-eval-method=source -verify %s 2>&1 \
+// RUN: | FileCheck %s --check-prefixes=CHECK-FE-ASSOC,CHECK-FE-RECPR,CHECK-FE-FUNC
+
+// RUN: not %clang_cc1 -fexperimental-strict-floating-point \
+// RUN: -triple x86_64-linux-gnu -ffp-eval-method=source \
+// RUN: -verify %s 2>&1 | FileCheck %s --check-prefixes=CHECK-FFP-OPT,CHECK-PRGM
+
+// CHECK-FE-FUNC: (frontend): option 'ffp-eval-method' cannot be used with option 'fapprox-func'.
+// CHECK-FE-ASSOC: (frontend): option 'ffp-eval-method' cannot be used with option 'mreassociate'.
+// CHECK-FE-RECPR: (frontend): option 'ffp-eval-method' cannot be used with option 'freciprocal'.
+
+// expected-no-diagnostics
+
+float f1(float a, float b, float c) {
+  a = b + c;
+  return a * b + c;
+}
+
+float f2(float a, float b, float c) {
+  // CHECK-FFP-OPT: option 'ffp-eval-method' cannot be used when '#pragma clang fp reassociate' is enabled.
+#pragma clang fp reassociate(on)
+  return (a + b) + c;
+}
+
+float f3(float a, float b, float c) {
+#pragma clang fp reassociate(off)
+  return (a - b) - c;
+}
+
+float f4(float a, float b, float c) {
+#pragma clang fp eval_method (double)
+  // CHECK-FUNC: '#pragma clang fp eval_method' cannot be used when option 'fapprox_func' is enabled.
+  // CHECK-ASSOC: '#pragma clang fp eval_method' cannot be used when option 'mreassociate' is enabled.
+  // CHECK-RECPR: '#pragma clang fp eval_method' cannot be used when option 'freciprocal' is enabled.
+  // CHECK-PRGM: '#pragma clang fp eval_method' cannot be used when '#pragma clang fp reassociate' is enabled.
+#pragma clang fp reassociate(on)
+  return (a * c) - (b * c);
+}
Index: clang/lib/Sema/SemaAttr.cpp
===
--- clang/lib/Sema/SemaAttr.cpp
+++ clang/lib/Sema/SemaAttr.cpp
@@ -486,6 +486,12 @@
 NewFPFeatures.setFPEvalMethodOverride(LangOptions::FEM_Extended);
 break;
   }
+  if (getLangOpts().ApproxFunc)
+Diag(Loc, diag::err_setting_eval_method_used_in_unsafe_context) << 0 << 0;
+  if (getLangOpts().AllowFPReassoc)
+Diag(Loc, diag::err_

[PATCH] D121302: [HIP] Fix -fno-gpu-sanitize

[Yaxun Liu via Phabricator via cfe-commits]

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



Comment at: clang/lib/Driver/ToolChains/HIPAMD.cpp:165
   // Diagnose unsupported sanitizer options only once.
+  if (!Args.hasFlag(options::OPT_fgpu_sanitize, options::OPT_fno_gpu_sanitize))
+return;

MaskRay wrote:
> Note: the third argument of hasFlag defaults to true before I removed it in 
> 522712e2d241ea33575a9c7a60ad582634f04f0d. I suspect it might not be what you 
> intended. 
The default value of OPT_fgpu_sanitize is intended to be true. Thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121302

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

[PATCH] D122588: [RISCV] Make PATH empty when testing --gcc-toolchain is multilib_riscv_elf_sdk

[luxufan via Phabricator via cfe-commits]

StephenFan created this revision.
StephenFan added reviewers: MaskRay, khchen, asb, jrtc27.
Herald added subscribers: s, VincentWu, luke957, vkmr, frasercrmck, evandro, 
luismarques, apazos, sameer.abuasal, s.egerton, Jim, benna, psnobl, jocewei, 
PkmX, the_o, brucehoult, MartinMosbeck, rogfer01, edward-jones, zzheng, 
kito-cheng, niosHD, sabuasal, simoncook, johnrusso, rbar, arichardson.
Herald added a project: All.
StephenFan requested review of this revision.
Herald added subscribers: cfe-commits, pcwang-thead, eopXD.
Herald added a project: clang.

Due to D79842 , clang dirver would search 
possible tool name in both possible
locations, then moving to the next name. The gcc toolchain 
`llvm-project/clang/test/Driver/
Inputs/multilib_riscv_elf_sdk` don't have a `riscv64-unknown-elf-ld` executable 
in
`llvm-project/clang/test/Driver/Inputs/multilib_riscv_elf_sdk/bin/`. So when 
searching
`riscv64-unknown-elf-ld`, if there is a `riscv64-unknown-elf-ld` in `PATH`, the
test would fail.

This patch makes the `PATH` empty when testing it.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D122588

Files:
  clang/test/Driver/riscv64-toolchain.c


Index: clang/test/Driver/riscv64-toolchain.c
===
--- clang/test/Driver/riscv64-toolchain.c
+++ clang/test/Driver/riscv64-toolchain.c
@@ -104,7 +104,7 @@
 // C-RV64-LINUX-MULTI-LP64D: 
"-L{{.*}}/Inputs/multilib_riscv_linux_sdk/sysroot/lib64/lp64d"
 // C-RV64-LINUX-MULTI-LP64D: 
"-L{{.*}}/Inputs/multilib_riscv_linux_sdk/sysroot/usr/lib64/lp64d"
 
-// RUN: %clang %s -### -fuse-ld=ld \
+// RUN: env "PATH=" %clang %s -### -fuse-ld=ld \
 // RUN:   --target=riscv64-unknown-elf --rtlib=platform --sysroot= \
 // RUN:   -march=rv64imac -mabi=lp64\
 // RUN:   --gcc-toolchain=%S/Inputs/multilib_riscv_elf_sdk 2>&1 \
@@ -119,7 +119,7 @@
 // C-RV64IMAC-BAREMETAL-MULTI-LP64: "--start-group" "-lc" "-lgloss" 
"--end-group" "-lgcc"
 // C-RV64IMAC-BAREMETAL-MULTI-LP64: 
"{{.*}}/Inputs/multilib_riscv_elf_sdk/lib/gcc/riscv64-unknown-elf/8.2.0/rv64imac/lp64{{/|}}crtend.o"
 
-// RUN: %clang %s -### -fuse-ld=ld \
+// RUN: env "PATH=" %clang %s -### -fuse-ld=ld \
 // RUN:   --target=riscv64-unknown-elf --rtlib=platform --sysroot= \
 // RUN:   -march=rv64imafdc -mabi=lp64d \
 // RUN:   --gcc-toolchain=%S/Inputs/multilib_riscv_elf_sdk 2>&1 \


Index: clang/test/Driver/riscv64-toolchain.c
===
--- clang/test/Driver/riscv64-toolchain.c
+++ clang/test/Driver/riscv64-toolchain.c
@@ -104,7 +104,7 @@
 // C-RV64-LINUX-MULTI-LP64D: "-L{{.*}}/Inputs/multilib_riscv_linux_sdk/sysroot/lib64/lp64d"
 // C-RV64-LINUX-MULTI-LP64D: "-L{{.*}}/Inputs/multilib_riscv_linux_sdk/sysroot/usr/lib64/lp64d"
 
-// RUN: %clang %s -### -fuse-ld=ld \
+// RUN: env "PATH=" %clang %s -### -fuse-ld=ld \
 // RUN:   --target=riscv64-unknown-elf --rtlib=platform --sysroot= \
 // RUN:   -march=rv64imac -mabi=lp64\
 // RUN:   --gcc-toolchain=%S/Inputs/multilib_riscv_elf_sdk 2>&1 \
@@ -119,7 +119,7 @@
 // C-RV64IMAC-BAREMETAL-MULTI-LP64: "--start-group" "-lc" "-lgloss" "--end-group" "-lgcc"
 // C-RV64IMAC-BAREMETAL-MULTI-LP64: "{{.*}}/Inputs/multilib_riscv_elf_sdk/lib/gcc/riscv64-unknown-elf/8.2.0/rv64imac/lp64{{/|}}crtend.o"
 
-// RUN: %clang %s -### -fuse-ld=ld \
+// RUN: env "PATH=" %clang %s -### -fuse-ld=ld \
 // RUN:   --target=riscv64-unknown-elf --rtlib=platform --sysroot= \
 // RUN:   -march=rv64imafdc -mabi=lp64d \
 // RUN:   --gcc-toolchain=%S/Inputs/multilib_riscv_elf_sdk 2>&1 \
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] e334f04 - [libTooling] Support TransformerResult in consumer callbacks

[Yitzhak Mandelbaum via cfe-commits]

Author: Eric Li
Date: 2022-03-28T15:39:46Z
New Revision: e334f044cdb5c0f4f549fc0380a6757e37ccb105

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

LOG: [libTooling] Support TransformerResult in consumer callbacks

Support `TransformerResult` in the consumer callback, which
allows generic code to more naturally use the `Transformer` interface
(instead of needing to specialize on `void`).

This also delete the specialization that existed within `Transformer`
itself, instead replacing it with an `std::function` adapter.

Reviewed By: ymandel

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

Added: 


Modified: 
clang/include/clang/Tooling/Transformer/Transformer.h
clang/lib/Tooling/Transformer/Transformer.cpp

Removed: 




diff  --git a/clang/include/clang/Tooling/Transformer/Transformer.h 
b/clang/include/clang/Tooling/Transformer/Transformer.h
index db2feac06d4aa..4e93783f2b9d9 100644
--- a/clang/include/clang/Tooling/Transformer/Transformer.h
+++ b/clang/include/clang/Tooling/Transformer/Transformer.h
@@ -21,7 +21,7 @@ namespace tooling {
 
 namespace detail {
 /// Implementation details of \c Transformer with type erasure around
-/// \c RewriteRule and \c RewriteRule as well as the corresponding 
consumers.
+/// \c RewriteRule as well as the corresponding consumers.
 class TransformerImpl {
 public:
   virtual ~TransformerImpl() = default;
@@ -43,33 +43,6 @@ class TransformerImpl {
   onMatchImpl(const ast_matchers::MatchFinder::MatchResult &Result) = 0;
 };
 
-/// Implementation for when no metadata is generated as a part of the
-/// \c RewriteRule.
-class NoMetadataImpl final : public TransformerImpl {
-  transformer::RewriteRule Rule;
-  std::function>)> Consumer;
-
-public:
-  explicit NoMetadataImpl(
-  transformer::RewriteRule R,
-  std::function>)>
-  Consumer)
-  : Rule(std::move(R)), Consumer(std::move(Consumer)) {
-assert(llvm::all_of(Rule.Cases,
-[](const transformer::RewriteRule::Case &Case) {
-  return Case.Edits;
-}) &&
-   "edit generator must be provided for each rule");
-  }
-
-private:
-  void onMatchImpl(const ast_matchers::MatchFinder::MatchResult &Result) final;
-  std::vector
-  buildMatchers() const final {
-return transformer::detail::buildMatchers(Rule);
-  }
-};
-
 // FIXME: Use std::type_identity or backport when available.
 template  struct type_identity {
   using type = T;
@@ -81,8 +54,6 @@ template  struct TransformerResult {
   T Metadata;
 };
 
-// Specialization provided only to avoid SFINAE on the Transformer
-// constructor; not intended for use.
 template <> struct TransformerResult {
   llvm::MutableArrayRef Changes;
 };
@@ -107,8 +78,14 @@ class Transformer : public 
ast_matchers::MatchFinder::MatchCallback {
   /// other.
   explicit Transformer(transformer::RewriteRuleWith Rule,
ChangeSetConsumer Consumer)
-  : Impl(std::make_unique(std::move(Rule),
-  std::move(Consumer))) {}
+  : Transformer(std::move(Rule),
+[Consumer = std::move(Consumer)](
+llvm::Expected> Result) {
+  if (Result)
+Consumer(Result->Changes);
+  else
+Consumer(Result.takeError());
+}) {}
 
   /// \param Consumer receives all rewrites and the associated metadata for a
   /// single match, or an error. Will always be called for each match, even if
@@ -135,6 +112,44 @@ class Transformer : public 
ast_matchers::MatchFinder::MatchCallback {
 };
 
 namespace detail {
+/// Asserts that all \c Metadata for the \c Rule is set.
+/// FIXME: Use constexpr-if in C++17.
+/// @{
+template 
+void assertMetadataSet(const transformer::RewriteRuleWith &Rule) {
+  assert(llvm::all_of(Rule.Metadata,
+  [](const typename transformer::Generator &Metadata)
+  -> bool { return !!Metadata; }) &&
+ "metadata generator must be provided for each rule");
+}
+template <>
+inline void assertMetadataSet(const transformer::RewriteRuleWith &) {}
+/// @}
+
+/// Runs the metadata generator on \c Rule and stuffs it into \c Result.
+/// FIXME: Use constexpr-if in C++17.
+/// @{
+template 
+llvm::Error
+populateMetadata(const transformer::RewriteRuleWith &Rule,
+ size_t SelectedCase,
+ const ast_matchers::MatchFinder::MatchResult &Match,
+ TransformerResult &Result) {
+  auto Metadata = Rule.Metadata[SelectedCase]->eval(Match);
+  if (!Metadata)
+return Metadata.takeError();
+  Result.Metadata = std::move(*Metadata);
+  return llvm::Error::success();
+

[PATCH] D122499: [libTooling] Support TransformerResult in consumer callbacks

[Yitzhak Mandelbaum 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 rGe334f044cdb5: [libTooling] Support 
TransformerResult in consumer callbacks (authored by li.zhe.hua, 
committed by ymandel).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122499

Files:
  clang/include/clang/Tooling/Transformer/Transformer.h
  clang/lib/Tooling/Transformer/Transformer.cpp

Index: clang/lib/Tooling/Transformer/Transformer.cpp
===
--- clang/lib/Tooling/Transformer/Transformer.cpp
+++ clang/lib/Tooling/Transformer/Transformer.cpp
@@ -66,26 +66,6 @@
   return Changes;
 }
 
-void NoMetadataImpl::onMatchImpl(const MatchFinder::MatchResult &Result) {
-  size_t I = transformer::detail::findSelectedCase(Result, Rule);
-  auto Transformations = Rule.Cases[I].Edits(Result);
-  if (!Transformations) {
-Consumer(Transformations.takeError());
-return;
-  }
-
-  if (Transformations->empty())
-return;
-
-  auto Changes = convertToAtomicChanges(*Transformations, Result);
-  if (!Changes) {
-Consumer(Changes.takeError());
-return;
-  }
-
-  Consumer(llvm::MutableArrayRef(*Changes));
-}
-
 } // namespace detail
 
 void Transformer::registerMatchers(MatchFinder *MatchFinder) {
Index: clang/include/clang/Tooling/Transformer/Transformer.h
===
--- clang/include/clang/Tooling/Transformer/Transformer.h
+++ clang/include/clang/Tooling/Transformer/Transformer.h
@@ -21,7 +21,7 @@
 
 namespace detail {
 /// Implementation details of \c Transformer with type erasure around
-/// \c RewriteRule and \c RewriteRule as well as the corresponding consumers.
+/// \c RewriteRule as well as the corresponding consumers.
 class TransformerImpl {
 public:
   virtual ~TransformerImpl() = default;
@@ -43,33 +43,6 @@
   onMatchImpl(const ast_matchers::MatchFinder::MatchResult &Result) = 0;
 };
 
-/// Implementation for when no metadata is generated as a part of the
-/// \c RewriteRule.
-class NoMetadataImpl final : public TransformerImpl {
-  transformer::RewriteRule Rule;
-  std::function>)> Consumer;
-
-public:
-  explicit NoMetadataImpl(
-  transformer::RewriteRule R,
-  std::function>)>
-  Consumer)
-  : Rule(std::move(R)), Consumer(std::move(Consumer)) {
-assert(llvm::all_of(Rule.Cases,
-[](const transformer::RewriteRule::Case &Case) {
-  return Case.Edits;
-}) &&
-   "edit generator must be provided for each rule");
-  }
-
-private:
-  void onMatchImpl(const ast_matchers::MatchFinder::MatchResult &Result) final;
-  std::vector
-  buildMatchers() const final {
-return transformer::detail::buildMatchers(Rule);
-  }
-};
-
 // FIXME: Use std::type_identity or backport when available.
 template  struct type_identity {
   using type = T;
@@ -81,8 +54,6 @@
   T Metadata;
 };
 
-// Specialization provided only to avoid SFINAE on the Transformer
-// constructor; not intended for use.
 template <> struct TransformerResult {
   llvm::MutableArrayRef Changes;
 };
@@ -107,8 +78,14 @@
   /// other.
   explicit Transformer(transformer::RewriteRuleWith Rule,
ChangeSetConsumer Consumer)
-  : Impl(std::make_unique(std::move(Rule),
-  std::move(Consumer))) {}
+  : Transformer(std::move(Rule),
+[Consumer = std::move(Consumer)](
+llvm::Expected> Result) {
+  if (Result)
+Consumer(Result->Changes);
+  else
+Consumer(Result.takeError());
+}) {}
 
   /// \param Consumer receives all rewrites and the associated metadata for a
   /// single match, or an error. Will always be called for each match, even if
@@ -135,6 +112,44 @@
 };
 
 namespace detail {
+/// Asserts that all \c Metadata for the \c Rule is set.
+/// FIXME: Use constexpr-if in C++17.
+/// @{
+template 
+void assertMetadataSet(const transformer::RewriteRuleWith &Rule) {
+  assert(llvm::all_of(Rule.Metadata,
+  [](const typename transformer::Generator &Metadata)
+  -> bool { return !!Metadata; }) &&
+ "metadata generator must be provided for each rule");
+}
+template <>
+inline void assertMetadataSet(const transformer::RewriteRuleWith &) {}
+/// @}
+
+/// Runs the metadata generator on \c Rule and stuffs it into \c Result.
+/// FIXME: Use constexpr-if in C++17.
+/// @{
+template 
+llvm::Error
+populateMetadata(const transformer::RewriteRuleWith &Rule,
+ size_t SelectedCase,
+ const ast_matchers::MatchFinder::MatchResult &Match,
+ TransformerResult &Result) {
+  auto Metadata 

[PATCH] D122589: Additionally set f32 mode with denormal-fp-math

[David Candler via Phabricator via cfe-commits]

dcandler created this revision.
dcandler added reviewers: arsenm, spatel.
Herald added a project: All.
dcandler requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay, wdng.
Herald added a project: clang.

When the denormal-fp-math option is used, this should set the
denormal handling mode for all floating point types. However,
currently 32-bit float types can ignore this setting as there is a
variant of the option, denormal-fp-math-f32, specifically for that type
which takes priority when checking the mode based on type and remains
at the default of IEEE. From the description, denormal-fp-math would
be expected to set the mode for floats unless overridden by the f32
variant, and code in the front end only emits the f32 option if it is
different to the general one, so setting just denormal-fp-math should
be valid.

This patch changes the denormal-fp-math option to also set the f32
mode. If denormal-fp-math-f32 is also specified, this is then
overridden as expected, but if it is absent floats will be set to the
mode specified by the former option, rather than remain on the default.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D122589

Files:
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp


Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -1497,7 +1497,8 @@
   if (Opts.FPDenormalMode != llvm::DenormalMode::getIEEE())
 GenerateArg(Args, OPT_fdenormal_fp_math_EQ, Opts.FPDenormalMode.str(), SA);
 
-  if (Opts.FP32DenormalMode != llvm::DenormalMode::getIEEE())
+  if ((Opts.FPDenormalMode != Opts.FP32DenormalMode) ||
+  (Opts.FP32DenormalMode != llvm::DenormalMode::getIEEE()))
 GenerateArg(Args, OPT_fdenormal_fp_math_f32_EQ, 
Opts.FP32DenormalMode.str(),
 SA);
 
@@ -1852,6 +1853,7 @@
   if (Arg *A = Args.getLastArg(OPT_fdenormal_fp_math_EQ)) {
 StringRef Val = A->getValue();
 Opts.FPDenormalMode = llvm::parseDenormalFPAttribute(Val);
+Opts.FP32DenormalMode = Opts.FPDenormalMode;
 if (!Opts.FPDenormalMode.isValid())
   Diags.Report(diag::err_drv_invalid_value) << A->getAsString(Args) << Val;
   }
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -2870,6 +2870,7 @@
 
 case options::OPT_fdenormal_fp_math_EQ:
   DenormalFPMath = llvm::parseDenormalFPAttribute(A->getValue());
+  DenormalFP32Math = DenormalFPMath;
   if (!DenormalFPMath.isValid()) {
 D.Diag(diag::err_drv_invalid_value)
 << A->getAsString(Args) << A->getValue();


Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -1497,7 +1497,8 @@
   if (Opts.FPDenormalMode != llvm::DenormalMode::getIEEE())
 GenerateArg(Args, OPT_fdenormal_fp_math_EQ, Opts.FPDenormalMode.str(), SA);
 
-  if (Opts.FP32DenormalMode != llvm::DenormalMode::getIEEE())
+  if ((Opts.FPDenormalMode != Opts.FP32DenormalMode) ||
+  (Opts.FP32DenormalMode != llvm::DenormalMode::getIEEE()))
 GenerateArg(Args, OPT_fdenormal_fp_math_f32_EQ, Opts.FP32DenormalMode.str(),
 SA);
 
@@ -1852,6 +1853,7 @@
   if (Arg *A = Args.getLastArg(OPT_fdenormal_fp_math_EQ)) {
 StringRef Val = A->getValue();
 Opts.FPDenormalMode = llvm::parseDenormalFPAttribute(Val);
+Opts.FP32DenormalMode = Opts.FPDenormalMode;
 if (!Opts.FPDenormalMode.isValid())
   Diags.Report(diag::err_drv_invalid_value) << A->getAsString(Args) << Val;
   }
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -2870,6 +2870,7 @@
 
 case options::OPT_fdenormal_fp_math_EQ:
   DenormalFPMath = llvm::parseDenormalFPAttribute(A->getValue());
+  DenormalFP32Math = DenormalFPMath;
   if (!DenormalFPMath.isValid()) {
 D.Diag(diag::err_drv_invalid_value)
 << A->getAsString(Args) << A->getValue();
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D122444: [Driver][Linux] Remove D.Dir+"/../lib" from default search paths for LLVM_ENABLE_RUNTIMES builds

[Johannes Doerfert via Phabricator via cfe-commits]

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

So, this broke all OpenMP (offloading) as libomp and (libomptarget) are no 
longer found by default. (OpenMP is build as ENABLE_RUNTIME.)

How do we fix this?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122444

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

[PATCH] D122444: [Driver][Linux] Remove D.Dir+"/../lib" from default search paths for LLVM_ENABLE_RUNTIMES builds

[Joseph Huber via Phabricator via cfe-commits]

jhuber6 added a comment.

This change broke finding the OpenMP runtime libraries when using the LLVM 
runtimes build.

  clang foo.c -fopenmp -fopenmp-targets=nvptx64
  /usr/lib64/gcc/x86_64-suse-linux/7/../../../../x86_64-suse-linux/bin/ld: 
cannot find -lomp
  /usr/lib64/gcc/x86_64-suse-linux/7/../../../../x86_64-suse-linux/bin/ld: 
cannot find -lomptarget


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122444

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

[PATCH] D122587: [clang][NFC] Extract the EmitAssemblyHelper::TargetTriple member

[Teresa Johnson via Phabricator via cfe-commits]

tejohnson accepted this revision.
tejohnson 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/D122587/new/

https://reviews.llvm.org/D122587

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

[PATCH] D122378: [doc] Rely on tblgen to dump supported options value when generating doc

[serge via Phabricator via cfe-commits]

serge-sans-paille updated this revision to Diff 418608.
serge-sans-paille added a comment.

Minor Options.td nits


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

https://reviews.llvm.org/D122378

Files:
  clang/include/clang/Driver/Options.td
  llvm/utils/TableGen/OptRSTEmitter.cpp

Index: llvm/utils/TableGen/OptRSTEmitter.cpp
===
--- llvm/utils/TableGen/OptRSTEmitter.cpp
+++ llvm/utils/TableGen/OptRSTEmitter.cpp
@@ -60,18 +60,45 @@
   // Print the option name.
   OS << R->getValueAsString("Name");
 
+  StringRef MetaVarName;
   // Print the meta-variable.
   if (!isa(R->getValueInit("MetaVarName"))) {
+MetaVarName = R->getValueAsString("MetaVarName");
+  } else if (!isa(R->getValueInit("Values")))
+MetaVarName = "";
+
+  if (!MetaVarName.empty()) {
 OS << '=';
-OS.write_escaped(R->getValueAsString("MetaVarName"));
+OS.write_escaped(MetaVarName);
   }
 
   OS << "\n\n";
 
+  std::string HelpText;
   // The option help text.
   if (!isa(R->getValueInit("HelpText"))) {
+HelpText = R->getValueAsString("HelpText").trim().str();
+if (!HelpText.empty() && HelpText.back() != '.')
+  HelpText.push_back('.');
+  }
+
+  if (!isa(R->getValueInit("Values"))) {
+SmallVector Values;
+SplitString(R->getValueAsString("Values"), Values, ",");
+HelpText += (" " + MetaVarName + " can be ").str();
+
+if (Values.size() == 1) {
+  HelpText += ("'" + Values.front() + "'.").str();
+} else {
+  HelpText += "one of '";
+  HelpText += join(Values.begin(), Values.end() - 1, "', '");
+  HelpText += ("' or '" + Values.back() + "'.").str();
+}
+  }
+
+  if (!HelpText.empty()) {
 OS << ' ';
-OS.write_escaped(R->getValueAsString("HelpText"));
+OS.write_escaped(HelpText);
 OS << "\n\n";
   }
 }
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -960,7 +960,7 @@
   PosFlag,
   NegFlag>;
 def fgpu_default_stream_EQ : Joined<["-"], "fgpu-default-stream=">,
-  HelpText<"Specify default stream. Valid values are 'legacy' and 'per-thread'. The default value is 'legacy'. (HIP only)">,
+  HelpText<"Specify default stream. The default value is 'legacy'. (HIP only)">,
   Flags<[CC1Option]>,
   Values<"legacy,per-thread">,
   NormalizedValuesScope<"LangOptions::GPUDefaultStreamKind">,
@@ -1171,7 +1171,7 @@
   MarshallingInfoStringVector>;
 def fembed_bitcode_EQ : Joined<["-"], "fembed-bitcode=">,
 Group, Flags<[NoXarchOption, CC1Option, CC1AsOption]>, MetaVarName<"">,
-HelpText<"Embed LLVM bitcode (option: off, all, bitcode, marker)">,
+HelpText<"Embed LLVM bitcode">,
 Values<"off,all,bitcode,marker">, NormalizedValuesScope<"CodeGenOptions">,
 NormalizedValues<["Embed_Off", "Embed_All", "Embed_Bitcode", "Embed_Marker"]>,
 MarshallingInfoEnum, "Embed_Off">;
@@ -1297,7 +1297,7 @@
 ShouldParseIf;
 def fprofile_update_EQ : Joined<["-"], "fprofile-update=">,
 Group, Flags<[CC1Option, CoreOption]>, Values<"atomic,prefer-atomic,single">,
-MetaVarName<"">, HelpText<"Set update method of profile counters (atomic,prefer-atomic,single)">,
+MetaVarName<"">, HelpText<"Set update method of profile counters">,
 MarshallingInfoFlag>;
 defm pseudo_probe_for_profiling : BoolFOption<"pseudo-probe-for-profiling",
   CodeGenOpts<"PseudoProbeForProfiling">, DefaultFalse,
@@ -1312,7 +1312,7 @@
 MarshallingInfoStringVector>;
 def fswift_async_fp_EQ : Joined<["-"], "fswift-async-fp=">,
 Group, Flags<[CC1Option, CC1AsOption, CoreOption]>, MetaVarName<"">,
-HelpText<"Control emission of Swift async extended frame info (option: auto, always, never)">,
+HelpText<"Control emission of Swift async extended frame info">,
 Values<"auto,always,never">,
 NormalizedValuesScope<"CodeGenOptions::SwiftAsyncFramePointerKind">,
 NormalizedValues<["Auto", "Always", "Never"]>,
@@ -1483,7 +1483,7 @@
 def fwasm_exceptions : Flag<["-"], "fwasm-exceptions">, Group,
   HelpText<"Use WebAssembly style exceptions">;
 def exception_model : Separate<["-"], "exception-model">,
-  Flags<[CC1Option, NoDriverOption]>, HelpText<"The exception model: dwarf|sjlj|seh|wasm">,
+  Flags<[CC1Option, NoDriverOption]>, HelpText<"The exception model">,
   Values<"dwarf,sjlj,seh,wasm">,
   NormalizedValuesScope<"LangOptions::ExceptionHandlingKind">,
   NormalizedValues<["DwarfCFI", "SjLj", "WinEH", "Wasm"]>,
@@ -1665,7 +1665,7 @@
   : Joined<["-"], "fsanitize-address-use-after-return=">,
 MetaVarName<"">,
 Flags<[CC1Option]>,
-HelpText<"Select the mode of detecting stack use-after-return in AddressSanitizer: never | runtime (default) | always">,
+HelpText<"Sel

[PATCH] D122546: Let clang-repl link privately against Clang components

[Vassil Vassilev via Phabricator via cfe-commits]

v.g.vassilev accepted this revision.
v.g.vassilev added a comment.
This revision is now accepted and ready to land.

Thanks for the clarification! LGTM!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122546

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

[PATCH] D122589: Additionally set f32 mode with denormal-fp-math

[Matt Arsenault via Phabricator via cfe-commits]

arsenm added a comment.

Testcase?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122589

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

[PATCH] D121812: [clang][deps] NFC: De-duplicate clang-cl tests

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 added a comment.

Thanks for the revert @gulfem. I was hoping I'll be able to quickly forward-fix 
the issue, but turns out there's more to investigate.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121812

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

[PATCH] D122592: [OpenMP] Fix library path missing when using OpenMP

[Joseph Huber via Phabricator via cfe-commits]

jhuber6 created this revision.
jhuber6 added reviewers: jdoerfert, JonChesterfield, ye-luo, MaskRay.
Herald added subscribers: StephenFan, guansong, yaxunl.
Herald added a project: All.
jhuber6 requested review of this revision.
Herald added subscribers: cfe-commits, sstefan1.
Herald added a project: clang.

The changes in D122444  caused OpenMP 
programs built with the
LLVM_ENABLE_RUNTIMES options to stop finding the libraries. We generally
expect to link against the libraries associated with the clang
installation itself but we no longer implicitly included that directory.
This patch adds in the include path of the clang installations library
to ensure we can find them.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D122592

Files:
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/lib/Driver/ToolChains/CommonArgs.h


Index: clang/lib/Driver/ToolChains/CommonArgs.h
===
--- clang/lib/Driver/ToolChains/CommonArgs.h
+++ clang/lib/Driver/ToolChains/CommonArgs.h
@@ -111,6 +111,9 @@
llvm::opt::ArgStringList &CmdArgs);
 void addArchSpecificRPath(const ToolChain &TC, const llvm::opt::ArgList &Args,
   llvm::opt::ArgStringList &CmdArgs);
+void addOpenMPRuntimeIncludePath(const ToolChain &TC,
+ const llvm::opt::ArgList &Args,
+ llvm::opt::ArgStringList &CmdArgs);
 /// Returns true, if an OpenMP runtime has been added.
 bool addOpenMPRuntime(llvm::opt::ArgStringList &CmdArgs, const ToolChain &TC,
   const llvm::opt::ArgList &Args,
Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -668,6 +668,17 @@
   }
 }
 
+void tools::addOpenMPRuntimeIncludePath(const ToolChain &TC,
+const ArgList &Args,
+ArgStringList &CmdArgs) {
+  // Default to clang lib / lib64 folder, i.e. the same location as device
+  // runtime.
+  SmallString<256> DefaultLibPath =
+  llvm::sys::path::parent_path(TC.getDriver().Dir);
+  llvm::sys::path::append(DefaultLibPath, Twine("lib") + CLANG_LIBDIR_SUFFIX);
+  CmdArgs.push_back(Args.MakeArgString("-L" + DefaultLibPath));
+}
+
 void tools::addArchSpecificRPath(const ToolChain &TC, const ArgList &Args,
  ArgStringList &CmdArgs) {
   // Enable -frtlib-add-rpath by default for the case of VE.
@@ -727,6 +738,7 @@
 
   if (RTKind == Driver::OMPRT_OMP)
 addOpenMPRuntimeSpecificRPath(TC, Args, CmdArgs);
+  addOpenMPRuntimeIncludePath(TC, Args, CmdArgs);
 
   return true;
 }


Index: clang/lib/Driver/ToolChains/CommonArgs.h
===
--- clang/lib/Driver/ToolChains/CommonArgs.h
+++ clang/lib/Driver/ToolChains/CommonArgs.h
@@ -111,6 +111,9 @@
llvm::opt::ArgStringList &CmdArgs);
 void addArchSpecificRPath(const ToolChain &TC, const llvm::opt::ArgList &Args,
   llvm::opt::ArgStringList &CmdArgs);
+void addOpenMPRuntimeIncludePath(const ToolChain &TC,
+ const llvm::opt::ArgList &Args,
+ llvm::opt::ArgStringList &CmdArgs);
 /// Returns true, if an OpenMP runtime has been added.
 bool addOpenMPRuntime(llvm::opt::ArgStringList &CmdArgs, const ToolChain &TC,
   const llvm::opt::ArgList &Args,
Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -668,6 +668,17 @@
   }
 }
 
+void tools::addOpenMPRuntimeIncludePath(const ToolChain &TC,
+const ArgList &Args,
+ArgStringList &CmdArgs) {
+  // Default to clang lib / lib64 folder, i.e. the same location as device
+  // runtime.
+  SmallString<256> DefaultLibPath =
+  llvm::sys::path::parent_path(TC.getDriver().Dir);
+  llvm::sys::path::append(DefaultLibPath, Twine("lib") + CLANG_LIBDIR_SUFFIX);
+  CmdArgs.push_back(Args.MakeArgString("-L" + DefaultLibPath));
+}
+
 void tools::addArchSpecificRPath(const ToolChain &TC, const ArgList &Args,
  ArgStringList &CmdArgs) {
   // Enable -frtlib-add-rpath by default for the case of VE.
@@ -727,6 +738,7 @@
 
   if (RTKind == Driver::OMPRT_OMP)
 addOpenMPRuntimeSpecificRPath(TC, Args, CmdArgs);
+  addOpenMPRuntimeIncludePath(TC, Args, CmdArgs);
 
   return true;
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D122592: [OpenMP] Fix library path missing when using OpenMP

[Joseph Huber via Phabricator via cfe-commits]

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

IncludePath -> LibraryPath


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122592

Files:
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/lib/Driver/ToolChains/CommonArgs.h


Index: clang/lib/Driver/ToolChains/CommonArgs.h
===
--- clang/lib/Driver/ToolChains/CommonArgs.h
+++ clang/lib/Driver/ToolChains/CommonArgs.h
@@ -111,6 +111,9 @@
llvm::opt::ArgStringList &CmdArgs);
 void addArchSpecificRPath(const ToolChain &TC, const llvm::opt::ArgList &Args,
   llvm::opt::ArgStringList &CmdArgs);
+void addOpenMPRuntimeLibraryPath(const ToolChain &TC,
+ const llvm::opt::ArgList &Args,
+ llvm::opt::ArgStringList &CmdArgs);
 /// Returns true, if an OpenMP runtime has been added.
 bool addOpenMPRuntime(llvm::opt::ArgStringList &CmdArgs, const ToolChain &TC,
   const llvm::opt::ArgList &Args,
Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -668,6 +668,17 @@
   }
 }
 
+void tools::addOpenMPRuntimeLibraryPath(const ToolChain &TC,
+const ArgList &Args,
+ArgStringList &CmdArgs) {
+  // Default to clang lib / lib64 folder, i.e. the same location as device
+  // runtime.
+  SmallString<256> DefaultLibPath =
+  llvm::sys::path::parent_path(TC.getDriver().Dir);
+  llvm::sys::path::append(DefaultLibPath, Twine("lib") + CLANG_LIBDIR_SUFFIX);
+  CmdArgs.push_back(Args.MakeArgString("-L" + DefaultLibPath));
+}
+
 void tools::addArchSpecificRPath(const ToolChain &TC, const ArgList &Args,
  ArgStringList &CmdArgs) {
   // Enable -frtlib-add-rpath by default for the case of VE.
@@ -727,6 +738,7 @@
 
   if (RTKind == Driver::OMPRT_OMP)
 addOpenMPRuntimeSpecificRPath(TC, Args, CmdArgs);
+  addOpenMPRuntimeLibraryPath(TC, Args, CmdArgs);
 
   return true;
 }


Index: clang/lib/Driver/ToolChains/CommonArgs.h
===
--- clang/lib/Driver/ToolChains/CommonArgs.h
+++ clang/lib/Driver/ToolChains/CommonArgs.h
@@ -111,6 +111,9 @@
llvm::opt::ArgStringList &CmdArgs);
 void addArchSpecificRPath(const ToolChain &TC, const llvm::opt::ArgList &Args,
   llvm::opt::ArgStringList &CmdArgs);
+void addOpenMPRuntimeLibraryPath(const ToolChain &TC,
+ const llvm::opt::ArgList &Args,
+ llvm::opt::ArgStringList &CmdArgs);
 /// Returns true, if an OpenMP runtime has been added.
 bool addOpenMPRuntime(llvm::opt::ArgStringList &CmdArgs, const ToolChain &TC,
   const llvm::opt::ArgList &Args,
Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -668,6 +668,17 @@
   }
 }
 
+void tools::addOpenMPRuntimeLibraryPath(const ToolChain &TC,
+const ArgList &Args,
+ArgStringList &CmdArgs) {
+  // Default to clang lib / lib64 folder, i.e. the same location as device
+  // runtime.
+  SmallString<256> DefaultLibPath =
+  llvm::sys::path::parent_path(TC.getDriver().Dir);
+  llvm::sys::path::append(DefaultLibPath, Twine("lib") + CLANG_LIBDIR_SUFFIX);
+  CmdArgs.push_back(Args.MakeArgString("-L" + DefaultLibPath));
+}
+
 void tools::addArchSpecificRPath(const ToolChain &TC, const ArgList &Args,
  ArgStringList &CmdArgs) {
   // Enable -frtlib-add-rpath by default for the case of VE.
@@ -727,6 +738,7 @@
 
   if (RTKind == Driver::OMPRT_OMP)
 addOpenMPRuntimeSpecificRPath(TC, Args, CmdArgs);
+  addOpenMPRuntimeLibraryPath(TC, Args, CmdArgs);
 
   return true;
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D122592: [OpenMP] Fix library path missing when using OpenMP

[Johannes Doerfert via Phabricator via cfe-commits]

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

s/include/library (commit message and code)

add test.

Assuming both are easy to fix, LG.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122592

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

[PATCH] D122592: [OpenMP] Fix library path missing when using OpenMP

[Fangrui Song via Phabricator via cfe-commits]

MaskRay accepted this revision.
MaskRay added a comment.

Another idea is to refactor `tools::addOpenMPRuntime` to use the full path of 
`libomp.so` or `libomp.a` instead of `-LXXX -lomp`.
The two ways have identical semantics when `libomp.so` has a `DT_SONAME` tag.

In the long term, perhaps libomp.so should be installed to the multiarch 
directory like `lib/x86_64-unknown-linux-gnu/libc++.so`.




Comment at: clang/lib/Driver/ToolChains/CommonArgs.cpp:741
 addOpenMPRuntimeSpecificRPath(TC, Args, CmdArgs);
+  addOpenMPRuntimeIncludePath(TC, Args, CmdArgs);
 

XXXIncludePath usually refers to the preprocessor include path.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122592

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

[clang] 16524d2 - [Driver][AVR] Fix warn_drv_avr_stdlib_not_linked condition

[Fangrui Song via cfe-commits]

Author: Fangrui Song
Date: 2022-03-28T09:43:36-07:00
New Revision: 16524d2f1bdc2e74ca34425edf43ac8ef5d39531

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

LOG: [Driver][AVR] Fix warn_drv_avr_stdlib_not_linked condition

Many options (-fsyntax-only, -E, -S, etc) skip the link action phase which the
existing condition does not account for.

Since the code no longer specifies OPT_c, I think a single RUN line about -c
not leading to a warning is sufficient. Adding one for all of -E,
-fsyntax-only, -S would be excessive.

Reviewed By: benshi001

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

Added: 


Modified: 
clang/lib/Driver/ToolChains/AVR.cpp
clang/lib/Driver/ToolChains/AVR.h
clang/test/Driver/avr-toolchain.c

Removed: 
clang/test/Driver/avr-link-no-mcu-specified.c



diff  --git a/clang/lib/Driver/ToolChains/AVR.cpp 
b/clang/lib/Driver/ToolChains/AVR.cpp
index 8688e06e78cc3..8ca8525e454b4 100644
--- a/clang/lib/Driver/ToolChains/AVR.cpp
+++ b/clang/lib/Driver/ToolChains/AVR.cpp
@@ -366,49 +366,19 @@ const StringRef PossibleAVRLibcLocations[] = {
 /// AVR Toolchain
 AVRToolChain::AVRToolChain(const Driver &D, const llvm::Triple &Triple,
const ArgList &Args)
-: Generic_ELF(D, Triple, Args), LinkStdlib(false) {
+: Generic_ELF(D, Triple, Args) {
   GCCInstallation.init(Triple, Args);
 
   // Only add default libraries if the user hasn't explicitly opted out.
   if (!Args.hasArg(options::OPT_nostdlib) &&
-  !Args.hasArg(options::OPT_nodefaultlibs) &&
-  !Args.hasArg(options::OPT_c /* does not apply when not linking */)) {
-std::string CPU = getCPUName(D, Args, Triple);
-
-if (CPU.empty()) {
-  // We cannot link any standard libraries without an MCU specified.
-  D.Diag(diag::warn_drv_avr_mcu_not_specified);
+  !Args.hasArg(options::OPT_nodefaultlibs)) {
+if (GCCInstallation.isValid()) {
+  GCCInstallPath = GCCInstallation.getInstallPath();
+  std::string GCCParentPath(GCCInstallation.getParentLibPath());
+  getProgramPaths().push_back(GCCParentPath + "/../bin");
 } else {
-  Optional FamilyName = GetMCUFamilyName(CPU);
-  Optional AVRLibcRoot = findAVRLibcInstallation();
-
-  if (!FamilyName.hasValue()) {
-// We do not have an entry for this CPU in the family
-// mapping table yet.
-D.Diag(diag::warn_drv_avr_family_linking_stdlibs_not_implemented)
-<< CPU;
-  } else if (!GCCInstallation.isValid()) {
-// No avr-gcc found and so no runtime linked.
-D.Diag(diag::warn_drv_avr_gcc_not_found);
-  } else if (!AVRLibcRoot.hasValue()) {
-// No avr-libc found and so no runtime linked.
-D.Diag(diag::warn_drv_avr_libc_not_found);
-  } else { // We have enough information to link stdlibs
-std::string GCCRoot(GCCInstallation.getInstallPath());
-std::string GCCParentPath(GCCInstallation.getParentLibPath());
-std::string LibcRoot = AVRLibcRoot.getValue();
-std::string SubPath = GetMCUSubPath(CPU);
-
-getProgramPaths().push_back(GCCParentPath + "/../bin");
-getFilePaths().push_back(LibcRoot + std::string("/lib/") + SubPath);
-getFilePaths().push_back(GCCRoot + std::string("/") + SubPath);
-
-LinkStdlib = true;
-  }
+  D.Diag(diag::warn_drv_avr_gcc_not_found);
 }
-
-if (!LinkStdlib)
-  D.Diag(diag::warn_drv_avr_stdlib_not_linked);
   }
 }
 
@@ -445,13 +415,14 @@ void AVRToolChain::addClangTargetOptions(
 }
 
 Tool *AVRToolChain::buildLinker() const {
-  return new tools::AVR::Linker(getTriple(), *this, LinkStdlib);
+  return new tools::AVR::Linker(getTriple(), *this);
 }
 
 void AVR::Linker::ConstructJob(Compilation &C, const JobAction &JA,
const InputInfo &Output,
const InputInfoList &Inputs, const ArgList 
&Args,
const char *LinkingOutput) const {
+  const auto &TC = static_cast(getToolChain());
   const Driver &D = getToolChain().getDriver();
 
   // Compute information about the target AVR.
@@ -473,6 +444,39 @@ void AVR::Linker::ConstructJob(Compilation &C, const 
JobAction &JA,
   Args.AddAllArgs(CmdArgs, options::OPT_L);
   getToolChain().AddFilePathLibArgs(Args, CmdArgs);
 
+  // Only add default libraries if the user hasn't explicitly opted out.
+  bool LinkStdlib = false;
+  if (!Args.hasArg(options::OPT_nostdlib) &&
+  !Args.hasArg(options::OPT_nodefaultlibs)) {
+if (CPU.empty()) {
+  // We cannot link any standard libraries without an MCU specified.
+  D.Diag(diag::warn_drv_avr_mcu_not_specified);
+} else {
+  Optional FamilyName = GetMCUFamilyName(CPU);
+  Optional AVRLibcRoot = TC.findAVRL

[PATCH] D122553: [Driver][AVR] Fix warn_drv_avr_stdlib_not_linked condition

[Fangrui Song 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 rG16524d2f1bdc: [Driver][AVR] Fix 
warn_drv_avr_stdlib_not_linked condition (authored by MaskRay).

Changed prior to commit:
  https://reviews.llvm.org/D122553?vs=418493&id=418623#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122553

Files:
  clang/lib/Driver/ToolChains/AVR.cpp
  clang/lib/Driver/ToolChains/AVR.h
  clang/test/Driver/avr-link-no-mcu-specified.c
  clang/test/Driver/avr-toolchain.c

Index: clang/test/Driver/avr-toolchain.c
===
--- clang/test/Driver/avr-toolchain.c
+++ clang/test/Driver/avr-toolchain.c
@@ -35,3 +35,11 @@
 // RUN: %clang %s -### -target avr --sysroot %S/Inputs/basic_avr_tree 2>&1 -nostdinc | FileCheck --check-prefix=NOSTDINC %s
 // RUN: %clang %s -### -target avr --sysroot %S/Inputs/basic_avr_tree 2>&1 -nostdlibinc | FileCheck --check-prefix=NOSTDINC %s
 // NOSTDINC-NOT: "-internal-isystem" {{".*avr/include"}}
+
+// RUN: %clang -### --target=avr %s 2>&1 | FileCheck --check-prefix=WARN_STDLIB %s
+// RUN: %clang -### --target=avr -mmcu=atmega328 %s 2>&1 | FileCheck --check-prefix=NOWARN_STDLIB %s
+// RUN: %clang -### --target=avr -c %s 2>&1 | FileCheck --check-prefix=NOWARN_STDLIB %s
+
+// WARN_STDLIB: warning: no target microcontroller specified on command line, cannot link standard libraries, please pass -mmcu=
+// WARN_STDLIB: warning: standard library not linked and so no interrupt vector table or compiler runtime routines will be linked
+// NOWARN_STDLIB-NOT: warning:
Index: clang/test/Driver/avr-link-no-mcu-specified.c
===
--- clang/test/Driver/avr-link-no-mcu-specified.c
+++ /dev/null
@@ -1,10 +0,0 @@
-// RUN: %clang -### -target avr -no-canonical-prefixes -save-temps %s 2>&1 | FileCheck --check-prefix=WARN %s
-// RUN: %clang -### -target avr -no-canonical-prefixes -save-temps -mmcu=atmega328 %s 2>&1 | FileCheck --check-prefix=NOWARN %s
-
-// WARN: warning: no target microcontroller specified on command line, cannot link standard libraries, please pass -mmcu=
-// WARN: warning: standard library not linked and so no interrupt vector table or compiler runtime routines will be linked
-
-// NOWARN: main
-
-int main() { return 0; }
-
Index: clang/lib/Driver/ToolChains/AVR.h
===
--- clang/lib/Driver/ToolChains/AVR.h
+++ clang/lib/Driver/ToolChains/AVR.h
@@ -31,17 +31,14 @@
 llvm::opt::ArgStringList &CC1Args,
 Action::OffloadKind DeviceOffloadKind) const override;
 
+  llvm::Optional findAVRLibcInstallation() const;
+  StringRef getGCCInstallPath() const { return GCCInstallPath; }
+
 protected:
   Tool *buildLinker() const override;
 
 private:
-  /// Whether libgcc, libct, and friends should be linked.
-  ///
-  /// This is not done if the user does not specify a
-  /// microcontroller on the command line.
-  bool LinkStdlib;
-
-  llvm::Optional findAVRLibcInstallation() const;
+  StringRef GCCInstallPath;
 };
 
 } // end namespace toolchains
@@ -50,9 +47,8 @@
 namespace AVR {
 class LLVM_LIBRARY_VISIBILITY Linker : public Tool {
 public:
-  Linker(const llvm::Triple &Triple, const ToolChain &TC, bool LinkStdlib)
-  : Tool("AVR::Linker", "avr-ld", TC), Triple(Triple),
-LinkStdlib(LinkStdlib) {}
+  Linker(const llvm::Triple &Triple, const ToolChain &TC)
+  : Tool("AVR::Linker", "avr-ld", TC), Triple(Triple) {}
 
   bool hasIntegratedCPP() const override { return false; }
   bool isLinkJob() const override { return true; }
@@ -63,7 +59,6 @@
 
 protected:
   const llvm::Triple &Triple;
-  bool LinkStdlib;
 };
 } // end namespace AVR
 } // end namespace tools
Index: clang/lib/Driver/ToolChains/AVR.cpp
===
--- clang/lib/Driver/ToolChains/AVR.cpp
+++ clang/lib/Driver/ToolChains/AVR.cpp
@@ -366,49 +366,19 @@
 /// AVR Toolchain
 AVRToolChain::AVRToolChain(const Driver &D, const llvm::Triple &Triple,
const ArgList &Args)
-: Generic_ELF(D, Triple, Args), LinkStdlib(false) {
+: Generic_ELF(D, Triple, Args) {
   GCCInstallation.init(Triple, Args);
 
   // Only add default libraries if the user hasn't explicitly opted out.
   if (!Args.hasArg(options::OPT_nostdlib) &&
-  !Args.hasArg(options::OPT_nodefaultlibs) &&
-  !Args.hasArg(options::OPT_c /* does not apply when not linking */)) {
-std::string CPU = getCPUName(D, Args, Triple);
-
-if (CPU.empty()) {
-  // We cannot link any standard libraries without an MCU specified.
-  D.Diag(diag::warn_drv_avr_mcu_not_specified);
+  !Args.hasArg(options::OPT_nodefaultlibs)) {
+if (GCCInstallation.isValid()) {
+  GCCInstallPath = GCC

[PATCH] D122553: [Driver][AVR] Fix warn_drv_avr_stdlib_not_linked condition

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added inline comments.



Comment at: clang/lib/Driver/ToolChains/AVR.cpp:451
+  !Args.hasArg(options::OPT_nodefaultlibs)) {
+if (CPU.empty()) {
+  // We cannot link any standard libraries without an MCU specified.

benshi001 wrote:
> I think we should warn empty CPU name in the compile stage. For example, if 
> user specified `-c` but not `-mmcu`, then we can not run to here to warn cpu 
> is empty.
> 
> So I will accept and rebase on your code.
The original code suggested that the diagnostic was intended for linking. As 
such, it should not fire for `-c`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122553

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

[PATCH] D122592: [OpenMP] Fix library path missing when using OpenMP

[Joseph Huber via Phabricator via cfe-commits]

jhuber6 added a comment.

In D122592#3411741 , @MaskRay wrote:

> Another idea is to refactor `tools::addOpenMPRuntime` to use the full path of 
> `libomp.so` or `libomp.a` instead of `-LXXX -lomp`.
> The two ways have identical semantics when `libomp.so` has a `DT_SONAME` tag.

It's technically possible for the runtime libraries to live somewhere else, I 
forget if any of our testers have a setup like that.

> In the long term, perhaps libomp.so should be installed to the multiarch 
> directory like `lib/x86_64-unknown-linux-gnu/libc++.so`.

Could you elaborate on the benefits of using the multiarch directory? I'm not 
familiar with it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122592

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

[PATCH] D122588: [RISCV] Make PATH empty when testing --gcc-toolchain is multilib_riscv_elf_sdk

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

Thanks for the patch. I think it's correct, though I'd add `[test]` to indicate 
this patch is test only and therefore not change any code.

If the test no longer works on Windows, add `// UNSUPPORTED: system-windows`

Does riscv32-toolchain.c need a similar change? Please look around for related 
tests when fixing something.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122588

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

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

[Teresa Johnson via Phabricator via cfe-commits]

tejohnson added inline comments.



Comment at: clang/lib/Frontend/CompilerInvocation.cpp:1554
+  Twine(*Opts.DiagnosticsMisExpectTolerance), SA);
+
   for (StringRef Sanitizer : serializeSanitizerKinds(Opts.SanitizeRecover))

paulkirth wrote:
> I really don't understand why this step is necessary, or why in my local 
> builds omitting the call to `GenerateArgs` works at all. I only arrived at 
> this change by noticing other options do the same, and this seemed to fix the 
> issue with tests using the repro instructions from the precommit bots.
> 
> It seems a bit strange that Clang parses the option, stores it to a 
> `CodeGenOption` then puts it back as a string argument to be parsed again 
> later and put into the same data structure. Is this a result of an earlier 
> architecture in Clang? if so, should we reconsider its design?
My understanding is that this is required as part of the cc1 command line 
parsing. Not sure why it worked in your local builds but not elsewhere. There 
is some info here: 
https://clang.llvm.org/docs/InternalsManual.html#adding-new-command-line-option


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115907

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

[PATCH] D122592: [OpenMP] Fix library path missing when using OpenMP

[Ye Luo via Phabricator via cfe-commits]

ye-luo added a comment.

> using the multiarch directory

If we can cross compile libomp and libomptarget to the target system. We may 
have
lib/x86_64-unknown-linux-gnu/libomp.so
lib/aarch64-unknown-linux-gnu/libomp.so
Compile clang once but compile runtime library for multiple architectures.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122592

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

[PATCH] D122592: [OpenMP] Fix library path missing when using OpenMP

[Joseph Huber via Phabricator via cfe-commits]

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

Add test.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122592

Files:
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/lib/Driver/ToolChains/CommonArgs.h
  clang/test/OpenMP/library-path.c


Index: clang/test/OpenMP/library-path.c
===
--- /dev/null
+++ clang/test/OpenMP/library-path.c
@@ -0,0 +1,6 @@
+// RUN: %clang -### -fopenmp=libomp -v %s 2>&1 | FileCheck %s
+
+void foo() {}
+
+// CHECK: InstalledDir: [[INSTALL:.*]]/{{.*}}
+// CHECK: -L[[INSTALL]]
Index: clang/lib/Driver/ToolChains/CommonArgs.h
===
--- clang/lib/Driver/ToolChains/CommonArgs.h
+++ clang/lib/Driver/ToolChains/CommonArgs.h
@@ -111,6 +111,9 @@
llvm::opt::ArgStringList &CmdArgs);
 void addArchSpecificRPath(const ToolChain &TC, const llvm::opt::ArgList &Args,
   llvm::opt::ArgStringList &CmdArgs);
+void addOpenMPRuntimeLibraryPath(const ToolChain &TC,
+ const llvm::opt::ArgList &Args,
+ llvm::opt::ArgStringList &CmdArgs);
 /// Returns true, if an OpenMP runtime has been added.
 bool addOpenMPRuntime(llvm::opt::ArgStringList &CmdArgs, const ToolChain &TC,
   const llvm::opt::ArgList &Args,
Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -668,6 +668,17 @@
   }
 }
 
+void tools::addOpenMPRuntimeLibraryPath(const ToolChain &TC,
+const ArgList &Args,
+ArgStringList &CmdArgs) {
+  // Default to clang lib / lib64 folder, i.e. the same location as device
+  // runtime.
+  SmallString<256> DefaultLibPath =
+  llvm::sys::path::parent_path(TC.getDriver().Dir);
+  llvm::sys::path::append(DefaultLibPath, Twine("lib") + CLANG_LIBDIR_SUFFIX);
+  CmdArgs.push_back(Args.MakeArgString("-L" + DefaultLibPath));
+}
+
 void tools::addArchSpecificRPath(const ToolChain &TC, const ArgList &Args,
  ArgStringList &CmdArgs) {
   // Enable -frtlib-add-rpath by default for the case of VE.
@@ -727,6 +738,7 @@
 
   if (RTKind == Driver::OMPRT_OMP)
 addOpenMPRuntimeSpecificRPath(TC, Args, CmdArgs);
+  addOpenMPRuntimeLibraryPath(TC, Args, CmdArgs);
 
   return true;
 }


Index: clang/test/OpenMP/library-path.c
===
--- /dev/null
+++ clang/test/OpenMP/library-path.c
@@ -0,0 +1,6 @@
+// RUN: %clang -### -fopenmp=libomp -v %s 2>&1 | FileCheck %s
+
+void foo() {}
+
+// CHECK: InstalledDir: [[INSTALL:.*]]/{{.*}}
+// CHECK: -L[[INSTALL]]
Index: clang/lib/Driver/ToolChains/CommonArgs.h
===
--- clang/lib/Driver/ToolChains/CommonArgs.h
+++ clang/lib/Driver/ToolChains/CommonArgs.h
@@ -111,6 +111,9 @@
llvm::opt::ArgStringList &CmdArgs);
 void addArchSpecificRPath(const ToolChain &TC, const llvm::opt::ArgList &Args,
   llvm::opt::ArgStringList &CmdArgs);
+void addOpenMPRuntimeLibraryPath(const ToolChain &TC,
+ const llvm::opt::ArgList &Args,
+ llvm::opt::ArgStringList &CmdArgs);
 /// Returns true, if an OpenMP runtime has been added.
 bool addOpenMPRuntime(llvm::opt::ArgStringList &CmdArgs, const ToolChain &TC,
   const llvm::opt::ArgList &Args,
Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -668,6 +668,17 @@
   }
 }
 
+void tools::addOpenMPRuntimeLibraryPath(const ToolChain &TC,
+const ArgList &Args,
+ArgStringList &CmdArgs) {
+  // Default to clang lib / lib64 folder, i.e. the same location as device
+  // runtime.
+  SmallString<256> DefaultLibPath =
+  llvm::sys::path::parent_path(TC.getDriver().Dir);
+  llvm::sys::path::append(DefaultLibPath, Twine("lib") + CLANG_LIBDIR_SUFFIX);
+  CmdArgs.push_back(Args.MakeArgString("-L" + DefaultLibPath));
+}
+
 void tools::addArchSpecificRPath(const ToolChain &TC, const ArgList &Args,
  ArgStringList &CmdArgs) {
   // Enable -frtlib-add-rpath by default for the case of VE.
@@ -727,6 +738,7 @@
 
   if (RTKind == Driver::OMPRT_OMP)
 addOpenMPRuntimeSpecificRPath(TC, Args, CmdArgs);
+  addOpenMPRuntimeLibraryPath(TC, Args, CmdArgs);
 
   return true;
 }
___

[PATCH] D122564: [RISCV] [NFC] add some tests for overloaded intrinsics of FP16

[Craig Topper via Phabricator via cfe-commits]

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

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122564

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

[PATCH] D122592: [OpenMP] Fix library path missing when using OpenMP

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

In D122592#3411795 , @ye-luo wrote:

>> using the multiarch directory
>
> If we can cross compile libomp and libomptarget to the target system. We may 
> have
> lib/x86_64-unknown-linux-gnu/libomp.so
> lib/aarch64-unknown-linux-gnu/libomp.so
> Compile clang once but compile runtime library for multiple architectures.

Yes, mostly for deambiguating cross compiling and multilib style -m32/-m64.
In addition, I think all LLVM_ENABLE_RUNTIMES projects but openmp switched to 
lib/$triple/xxx.so for runtime libraries, e.g. libc++.so, libc++abi.so, 
libunwind.a.
By switching openmp we will improve consistency. Downstream distributions can 
use the same way to install runtime libraries, instead of doing different 
things for openmp and non-openmp.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122592

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

[PATCH] D122592: [OpenMP] Fix library path missing when using OpenMP

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added inline comments.



Comment at: clang/test/OpenMP/library-path.c:1
+// RUN: %clang -### -fopenmp=libomp -v %s 2>&1 | FileCheck %s
+

Suggest the style in `linux-cross.cpp`: use `-ccc-install-dir 
%S/Inputs/basic_linux_tree/usr/bin` to specify InstalledDir and remove `-v`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122592

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

[PATCH] D122592: [OpenMP] Fix library path missing when using OpenMP

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added inline comments.



Comment at: clang/test/OpenMP/library-path.c:1
+// RUN: %clang -### -fopenmp=libomp -v %s 2>&1 | FileCheck %s
+

MaskRay wrote:
> Suggest the style in `linux-cross.cpp`: use `-ccc-install-dir 
> %S/Inputs/basic_linux_tree/usr/bin` to specify InstalledDir and remove `-v`.
Without a `--target=` I suspect this may fail on some targets.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122592

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

[clang] 17ea5bf - Revert "[Driver][AVR] Fix warn_drv_avr_stdlib_not_linked condition"

[Fangrui Song via cfe-commits]

Author: Fangrui Song
Date: 2022-03-28T10:11:45-07:00
New Revision: 17ea5bf1d54d13d6505b636e56ab151a45eab12d

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

LOG: Revert "[Driver][AVR] Fix warn_drv_avr_stdlib_not_linked condition"

This reverts commit 16524d2f1bdc2e74ca34425edf43ac8ef5d39531.

The test caused some warnings when avr-gcc was not installed.

Added: 
clang/test/Driver/avr-link-no-mcu-specified.c

Modified: 
clang/lib/Driver/ToolChains/AVR.cpp
clang/lib/Driver/ToolChains/AVR.h
clang/test/Driver/avr-toolchain.c

Removed: 




diff  --git a/clang/lib/Driver/ToolChains/AVR.cpp 
b/clang/lib/Driver/ToolChains/AVR.cpp
index 8ca8525e454b4..8688e06e78cc3 100644
--- a/clang/lib/Driver/ToolChains/AVR.cpp
+++ b/clang/lib/Driver/ToolChains/AVR.cpp
@@ -366,19 +366,49 @@ const StringRef PossibleAVRLibcLocations[] = {
 /// AVR Toolchain
 AVRToolChain::AVRToolChain(const Driver &D, const llvm::Triple &Triple,
const ArgList &Args)
-: Generic_ELF(D, Triple, Args) {
+: Generic_ELF(D, Triple, Args), LinkStdlib(false) {
   GCCInstallation.init(Triple, Args);
 
   // Only add default libraries if the user hasn't explicitly opted out.
   if (!Args.hasArg(options::OPT_nostdlib) &&
-  !Args.hasArg(options::OPT_nodefaultlibs)) {
-if (GCCInstallation.isValid()) {
-  GCCInstallPath = GCCInstallation.getInstallPath();
-  std::string GCCParentPath(GCCInstallation.getParentLibPath());
-  getProgramPaths().push_back(GCCParentPath + "/../bin");
+  !Args.hasArg(options::OPT_nodefaultlibs) &&
+  !Args.hasArg(options::OPT_c /* does not apply when not linking */)) {
+std::string CPU = getCPUName(D, Args, Triple);
+
+if (CPU.empty()) {
+  // We cannot link any standard libraries without an MCU specified.
+  D.Diag(diag::warn_drv_avr_mcu_not_specified);
 } else {
-  D.Diag(diag::warn_drv_avr_gcc_not_found);
+  Optional FamilyName = GetMCUFamilyName(CPU);
+  Optional AVRLibcRoot = findAVRLibcInstallation();
+
+  if (!FamilyName.hasValue()) {
+// We do not have an entry for this CPU in the family
+// mapping table yet.
+D.Diag(diag::warn_drv_avr_family_linking_stdlibs_not_implemented)
+<< CPU;
+  } else if (!GCCInstallation.isValid()) {
+// No avr-gcc found and so no runtime linked.
+D.Diag(diag::warn_drv_avr_gcc_not_found);
+  } else if (!AVRLibcRoot.hasValue()) {
+// No avr-libc found and so no runtime linked.
+D.Diag(diag::warn_drv_avr_libc_not_found);
+  } else { // We have enough information to link stdlibs
+std::string GCCRoot(GCCInstallation.getInstallPath());
+std::string GCCParentPath(GCCInstallation.getParentLibPath());
+std::string LibcRoot = AVRLibcRoot.getValue();
+std::string SubPath = GetMCUSubPath(CPU);
+
+getProgramPaths().push_back(GCCParentPath + "/../bin");
+getFilePaths().push_back(LibcRoot + std::string("/lib/") + SubPath);
+getFilePaths().push_back(GCCRoot + std::string("/") + SubPath);
+
+LinkStdlib = true;
+  }
 }
+
+if (!LinkStdlib)
+  D.Diag(diag::warn_drv_avr_stdlib_not_linked);
   }
 }
 
@@ -415,14 +445,13 @@ void AVRToolChain::addClangTargetOptions(
 }
 
 Tool *AVRToolChain::buildLinker() const {
-  return new tools::AVR::Linker(getTriple(), *this);
+  return new tools::AVR::Linker(getTriple(), *this, LinkStdlib);
 }
 
 void AVR::Linker::ConstructJob(Compilation &C, const JobAction &JA,
const InputInfo &Output,
const InputInfoList &Inputs, const ArgList 
&Args,
const char *LinkingOutput) const {
-  const auto &TC = static_cast(getToolChain());
   const Driver &D = getToolChain().getDriver();
 
   // Compute information about the target AVR.
@@ -444,39 +473,6 @@ void AVR::Linker::ConstructJob(Compilation &C, const 
JobAction &JA,
   Args.AddAllArgs(CmdArgs, options::OPT_L);
   getToolChain().AddFilePathLibArgs(Args, CmdArgs);
 
-  // Only add default libraries if the user hasn't explicitly opted out.
-  bool LinkStdlib = false;
-  if (!Args.hasArg(options::OPT_nostdlib) &&
-  !Args.hasArg(options::OPT_nodefaultlibs)) {
-if (CPU.empty()) {
-  // We cannot link any standard libraries without an MCU specified.
-  D.Diag(diag::warn_drv_avr_mcu_not_specified);
-} else {
-  Optional FamilyName = GetMCUFamilyName(CPU);
-  Optional AVRLibcRoot = TC.findAVRLibcInstallation();
-
-  if (!FamilyName) {
-// We do not have an entry for this CPU in the family
-// mapping table yet.
-D.Diag(diag::warn_drv_avr_family_linking_stdlibs_not_implemented)
-<< CPU;
-  } els

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

[Eli Friedman via Phabricator via cfe-commits]

efriedma added inline comments.



Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:1329
 setOperationAction(ISD::INTRINSIC_VOID, MVT::i128, Custom);
   }
 

You probably want an "else" here to call `setMaxAtomicSizeInBitsSupported(64)` 
or something like that.  But you can leave that to a followup patch if you want.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122377

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

[PATCH] D122524: [clang][AVR] Emit proper warnings

[Ben Shi via Phabricator via cfe-commits]

benshi001 updated this revision to Diff 418634.
benshi001 marked an inline comment as done.
benshi001 retitled this revision from "[clang][AVR] Generate link warnings 
properly" to "[clang][AVR] Emit proper warnings".
benshi001 edited the summary of this revision.
benshi001 set the repository for this revision to rG LLVM Github Monorepo.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122524

Files:
  clang/lib/Driver/ToolChains/AVR.cpp
  clang/test/Driver/avr-toolchain.c


Index: clang/test/Driver/avr-toolchain.c
===
--- clang/test/Driver/avr-toolchain.c
+++ clang/test/Driver/avr-toolchain.c
@@ -36,10 +36,38 @@
 // RUN: %clang %s -### -target avr --sysroot %S/Inputs/basic_avr_tree 2>&1 
-nostdlibinc | FileCheck --check-prefix=NOSTDINC %s
 // NOSTDINC-NOT: "-internal-isystem" {{".*avr/include"}}
 
-// RUN: %clang -### --target=avr %s 2>&1 | FileCheck 
--check-prefix=WARN_STDLIB %s
-// RUN: %clang -### --target=avr -mmcu=atmega328 %s 2>&1 | FileCheck 
--check-prefix=NOWARN_STDLIB %s
-// RUN: %clang -### --target=avr -c %s 2>&1 | FileCheck 
--check-prefix=NOWARN_STDLIB %s
+// RUN: %clang -### --target=avr %s --sysroot %S/Inputs/basic_avr_tree 
-mmcu=atmega328 2>&1 | FileCheck --check-prefix=NOWARN %s
+// RUN: %clang -### --target=avr %s --sysroot %S/Inputs/basic_avr_tree 
-mmcu=atmega328 -S 2>&1 | FileCheck --check-prefix=NOWARN %s
+// NOWARN-NOT: warning:
 
-// WARN_STDLIB: warning: no target microcontroller specified on command line, 
cannot link standard libraries, please pass -mmcu=
-// WARN_STDLIB: warning: standard library not linked and so no interrupt 
vector table or compiler runtime routines will be linked
-// NOWARN_STDLIB-NOT: warning:
+// RUN: %clang -### --target=avr %s --sysroot %S/Inputs -S 2>&1 | FileCheck 
--check-prefixes=NOLINKA,NOLINK %s
+// RUN: %clang -### --target=avr %s --sysroot %S/Inputs/basic_avr_tree -S 2>&1 
| FileCheck --check-prefixes=NOLINKB,NOLINK %s
+// RUN: %clang -### --target=avr %s --sysroot %S/Inputs -mmcu=atmega328 -S 
2>&1 | FileCheck --check-prefixes=NOLINKC,NOLINK %s
+// NOLINKA: warning: no target microcontroller specified on command line, 
cannot link standard libraries, please pass -mmcu=
+// NOLINKA: warning: no avr-gcc installation can be found on the system, 
cannot link standard libraries
+// NOLINKB: warning: no target microcontroller specified on command line, 
cannot link standard libraries, please pass -mmcu=
+// NOLINKC: warning: no avr-gcc installation can be found on the system, 
cannot link standard libraries
+// NOLINK-NOT: warning: {{.*}} avr-libc
+// NOLINK-NOT: warning: {{.*}} interrupt vector
+// NOLINK-NOT: warning: {{.*}} data section address
+// NOLINKB-NOT: warning: {{.*}} microcontroller
+// NOLINKC-NOT: warning: {{.*}} avr-gcc
+
+// RUN: %clang -### --target=avr %s --sysroot %S/Inputs 2>&1 | FileCheck 
--check-prefixes=LINKA1,LINKA %s
+// RUN: %clang -### --target=avr %s --sysroot %S/Inputs/basic_avr_tree 2>&1 | 
FileCheck --check-prefixes=LINKA2,LINKA %s
+// LINKA: warning: no target microcontroller specified on command line, cannot 
link standard libraries, please pass -mmcu=
+// LINKA1: warning: no avr-gcc installation can be found on the system, cannot 
link standard libraries
+// LINKA: warning: standard library not linked and so no interrupt vector 
table or compiler runtime routines will be linked
+// LINKA: support for passing the data section address to the linker for 
microcontroller '' is not implemented
+// LINKA2-NOT: warning: {{.*}} avr-gcc
+// LINKA-NOT: warning: {{.*}} avr-libc
+
+// RUN: %clang -### --target=avr %s --sysroot %S/Inputs 2>&1 | FileCheck 
--check-prefixes=LINKB1,LINKB %s
+// LINKB1: warning: no target microcontroller specified on command line, 
cannot link standard libraries, please pass -mmcu=
+// LINKB: warning: no avr-gcc installation can be found on the system, cannot 
link standard libraries
+// LINKB2: warning: no avr-libc installation can be found on the system, 
cannot link standard libraries
+// LINKB: warning: standard library not linked and so no interrupt vector 
table or compiler runtime routines will be linked
+// LINKB1: support for passing the data section address to the linker for 
microcontroller '' is not implemented
+// LINKB2-NOT: warning: {{.*}} microcontroller
+// LINKB1-NOT: warning: {{.*}} avr-libc
+// LINKB2-NOT: warning: {{.*}} data section address
Index: clang/lib/Driver/ToolChains/AVR.cpp
===
--- clang/lib/Driver/ToolChains/AVR.cpp
+++ clang/lib/Driver/ToolChains/AVR.cpp
@@ -369,6 +369,10 @@
 : Generic_ELF(D, Triple, Args) {
   GCCInstallation.init(Triple, Args);
 
+  std::string CPU = getCPUName(D, Args, Triple);
+  if (CPU.empty())
+D.Diag(diag::warn_drv_avr_mcu_not_specified);
+
   // Only add default libraries if the user hasn't explicitly opted out.
   if (!A

[PATCH] D121951: [AMDGPU][OpenCL] Add "amdgpu-no-hostcall-ptr" in Clang codegen pre-COV_5

[Scott Linder via Phabricator via cfe-commits]

scott.linder updated this revision to Diff 418635.
scott.linder added a comment.

Do not add the no-hostcall attribute for "COV_None" modules, to allow for
the OpenCL implementation of hostcall in the device-libs.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121951

Files:
  clang/lib/CodeGen/TargetInfo.cpp
  clang/test/CodeGenOpenCL/amdgpu-attrs.cl
  clang/test/CodeGenOpenCL/amdgpu-printf.cl
  llvm/docs/AMDGPUUsage.rst
  llvm/lib/Target/AMDGPU/AMDGPUHSAMetadataStreamer.cpp
  llvm/lib/Target/AMDGPU/AMDGPUPrintfRuntimeBinding.cpp
  llvm/test/CodeGen/AMDGPU/hsa-metadata-from-llvm-ir-full-v3.ll
  llvm/test/CodeGen/AMDGPU/opencl-printf-and-hostcall.ll
  llvm/test/CodeGen/AMDGPU/opencl-printf-no-hostcall.ll

Index: llvm/test/CodeGen/AMDGPU/opencl-printf-no-hostcall.ll
===
--- llvm/test/CodeGen/AMDGPU/opencl-printf-no-hostcall.ll
+++ /dev/null
@@ -1,18 +0,0 @@
-; RUN: not opt -S -mtriple=amdgcn-unknown-unknown -amdgpu-printf-runtime-binding < %s 2>&1 | FileCheck %s
-
-@.str = private unnamed_addr addrspace(2) constant [6 x i8] c"%s:%d\00", align 1
-
-define amdgpu_kernel void @test_kernel(i32 %n) {
-entry:
-  %str = alloca [9 x i8], align 1
-  %arraydecay = getelementptr inbounds [9 x i8], [9 x i8]* %str, i32 0, i32 0
-  %call1 = call i32 (i8 addrspace(2)*, ...) @printf(i8 addrspace(2)* getelementptr inbounds ([6 x i8], [6 x i8] addrspace(2)* @.str, i32 0, i32 0), i8* %arraydecay, i32 %n)
-  %call2 = call <2 x i64> (i8*, i32, i64, i64, i64, i64, i64, i64, i64, i64) @__ockl_hostcall_internal(i8* undef, i32 1, i64 2, i64 3, i64 4, i64 5, i64 6, i64 7, i64 8, i64 9)
-  ret void
-}
-
-declare i32 @printf(i8 addrspace(2)*, ...)
-
-declare <2 x i64> @__ockl_hostcall_internal(i8*, i32, i64, i64, i64, i64, i64, i64, i64, i64)
-
-; CHECK: error: Cannot use both printf and hostcall in the same module
Index: llvm/test/CodeGen/AMDGPU/opencl-printf-and-hostcall.ll
===
--- /dev/null
+++ llvm/test/CodeGen/AMDGPU/opencl-printf-and-hostcall.ll
@@ -0,0 +1,19 @@
+; RUN: opt -S -mtriple=amdgcn-unknown-unknown -amdgpu-printf-runtime-binding < %s 2>&1 | FileCheck %s
+
+@.str = private unnamed_addr addrspace(4) constant [6 x i8] c"%s:%d\00", align 1
+
+define amdgpu_kernel void @test_kernel(i32 %n) {
+entry:
+  %str = alloca [9 x i8], align 1, addrspace(5)
+  %arraydecay = getelementptr inbounds [9 x i8], [9 x i8] addrspace(5)* %str, i32 0, i32 0
+  %call1 = call i32 (i8 addrspace(4)*, ...) @printf(i8 addrspace(4)* getelementptr inbounds ([6 x i8], [6 x i8] addrspace(4)* @.str, i32 0, i32 0), i8 addrspace(5)* %arraydecay, i32 %n)
+  %call2 = call <2 x i64> (i8*, i32, i64, i64, i64, i64, i64, i64, i64, i64) @__ockl_hostcall_internal(i8* undef, i32 1, i64 2, i64 3, i64 4, i64 5, i64 6, i64 7, i64 8, i64 9)
+  ret void
+}
+
+declare i32 @printf(i8 addrspace(4)*, ...)
+
+declare <2 x i64> @__ockl_hostcall_internal(i8*, i32, i64, i64, i64, i64, i64, i64, i64, i64)
+
+; CHECK-NOT: error:
+; CHECK-NOT: warning:
Index: llvm/test/CodeGen/AMDGPU/hsa-metadata-from-llvm-ir-full-v3.ll
===
--- llvm/test/CodeGen/AMDGPU/hsa-metadata-from-llvm-ir-full-v3.ll
+++ llvm/test/CodeGen/AMDGPU/hsa-metadata-from-llvm-ir-full-v3.ll
@@ -1894,9 +1894,9 @@
 ; CHECK-NEXT: - 1
 ; CHECK-NEXT: - 0
 
-attributes #0 = { optnone noinline "amdgpu-implicitarg-num-bytes"="56" }
-attributes #1 = { optnone noinline "amdgpu-implicitarg-num-bytes"="56" "runtime-handle"="__test_block_invoke_kernel_runtime_handle" }
-attributes #2 = { optnone noinline "amdgpu-implicitarg-num-bytes"="56" "calls-enqueue-kernel" }
+attributes #0 = { optnone noinline "amdgpu-implicitarg-num-bytes"="56" "amdgpu-no-hostcall-ptr" }
+attributes #1 = { optnone noinline "amdgpu-implicitarg-num-bytes"="56" "amdgpu-no-hostcall-ptr" "runtime-handle"="__test_block_invoke_kernel_runtime_handle" }
+attributes #2 = { optnone noinline "amdgpu-implicitarg-num-bytes"="56" "amdgpu-no-hostcall-ptr" "calls-enqueue-kernel" }
 
 !llvm.printf.fmts = !{!100, !101}
 
Index: llvm/lib/Target/AMDGPU/AMDGPUPrintfRuntimeBinding.cpp
===
--- llvm/lib/Target/AMDGPU/AMDGPUPrintfRuntimeBinding.cpp
+++ llvm/lib/Target/AMDGPU/AMDGPUPrintfRuntimeBinding.cpp
@@ -563,15 +563,6 @@
   if (Printfs.empty())
 return false;
 
-  if (auto HostcallFunction = M.getFunction("__ockl_hostcall_internal")) {
-for (auto &U : HostcallFunction->uses()) {
-  if (auto *CI = dyn_cast(U.getUser())) {
-M.getContext().emitError(
-CI, "Cannot use both printf and hostcall in the same module");
-  }
-}
-  }
-
   TD = &M.getDataLayout();
 
   return lowerPrintfForGpu(M);
Index: llvm/lib/Target/AMDGPU/AMDGPUHSAMetadataStreamer.cpp
==

[PATCH] D121951: [AMDGPU][OpenCL] Add "amdgpu-no-hostcall-ptr" in Clang codegen pre-COV_5

[Scott Linder via Phabricator via cfe-commits]

scott.linder added a comment.

@yaxunl Does excluding device-libs via COV_None make sense?

@sameerds Would you still rather we just not add this attribute in the frontend 
at all? I'm OK with it if that is the consensus


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121951

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

[PATCH] D122524: [clang][AVR] Emit proper warnings

[Ben Shi via Phabricator via cfe-commits]

benshi001 added a comment.

In D122524#3410542 , @MaskRay wrote:

> I think it is excessive to add so many RUN lines. I do not understand much 
> about AVR -mcpu. That said, I created D122553 
>  for what I think should be done for the 
> `-c/-S/-fsyntax-only` condition. More tests would just be excessive.
>
> You may adjust the patch to do the rest cleanups/fixes.

I have made some changes based on your https://reviews.llvm.org/D122553, it 
seems you have reverted. I suggest you recommit, since I have fixed the 
failures of lacking avr-gcc.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122524

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

[PATCH] D122592: [OpenMP] Fix library path missing when using OpenMP

[Joseph Huber via Phabricator via cfe-commits]

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

Making suggested changes to test.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122592

Files:
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/lib/Driver/ToolChains/CommonArgs.h
  clang/test/Driver/openmp-library-path.c


Index: clang/test/Driver/openmp-library-path.c
===
--- /dev/null
+++ clang/test/Driver/openmp-library-path.c
@@ -0,0 +1,5 @@
+// RUN: %clang -### -fopenmp=libomp --target=x86_64-linux-gnu -ccc-install-dir 
%S/Inputs/basic_linux_tree/usr/bin %s 2>&1 | FileCheck %s
+
+void foo() {}
+
+// CHECK: -L{{.*}}Inputs{{.*}}basic_linux_tree
Index: clang/lib/Driver/ToolChains/CommonArgs.h
===
--- clang/lib/Driver/ToolChains/CommonArgs.h
+++ clang/lib/Driver/ToolChains/CommonArgs.h
@@ -111,6 +111,9 @@
llvm::opt::ArgStringList &CmdArgs);
 void addArchSpecificRPath(const ToolChain &TC, const llvm::opt::ArgList &Args,
   llvm::opt::ArgStringList &CmdArgs);
+void addOpenMPRuntimeLibraryPath(const ToolChain &TC,
+ const llvm::opt::ArgList &Args,
+ llvm::opt::ArgStringList &CmdArgs);
 /// Returns true, if an OpenMP runtime has been added.
 bool addOpenMPRuntime(llvm::opt::ArgStringList &CmdArgs, const ToolChain &TC,
   const llvm::opt::ArgList &Args,
Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -668,6 +668,17 @@
   }
 }
 
+void tools::addOpenMPRuntimeLibraryPath(const ToolChain &TC,
+const ArgList &Args,
+ArgStringList &CmdArgs) {
+  // Default to clang lib / lib64 folder, i.e. the same location as device
+  // runtime.
+  SmallString<256> DefaultLibPath =
+  llvm::sys::path::parent_path(TC.getDriver().Dir);
+  llvm::sys::path::append(DefaultLibPath, Twine("lib") + CLANG_LIBDIR_SUFFIX);
+  CmdArgs.push_back(Args.MakeArgString("-L" + DefaultLibPath));
+}
+
 void tools::addArchSpecificRPath(const ToolChain &TC, const ArgList &Args,
  ArgStringList &CmdArgs) {
   // Enable -frtlib-add-rpath by default for the case of VE.
@@ -727,6 +738,7 @@
 
   if (RTKind == Driver::OMPRT_OMP)
 addOpenMPRuntimeSpecificRPath(TC, Args, CmdArgs);
+  addOpenMPRuntimeLibraryPath(TC, Args, CmdArgs);
 
   return true;
 }


Index: clang/test/Driver/openmp-library-path.c
===
--- /dev/null
+++ clang/test/Driver/openmp-library-path.c
@@ -0,0 +1,5 @@
+// RUN: %clang -### -fopenmp=libomp --target=x86_64-linux-gnu -ccc-install-dir %S/Inputs/basic_linux_tree/usr/bin %s 2>&1 | FileCheck %s
+
+void foo() {}
+
+// CHECK: -L{{.*}}Inputs{{.*}}basic_linux_tree
Index: clang/lib/Driver/ToolChains/CommonArgs.h
===
--- clang/lib/Driver/ToolChains/CommonArgs.h
+++ clang/lib/Driver/ToolChains/CommonArgs.h
@@ -111,6 +111,9 @@
llvm::opt::ArgStringList &CmdArgs);
 void addArchSpecificRPath(const ToolChain &TC, const llvm::opt::ArgList &Args,
   llvm::opt::ArgStringList &CmdArgs);
+void addOpenMPRuntimeLibraryPath(const ToolChain &TC,
+ const llvm::opt::ArgList &Args,
+ llvm::opt::ArgStringList &CmdArgs);
 /// Returns true, if an OpenMP runtime has been added.
 bool addOpenMPRuntime(llvm::opt::ArgStringList &CmdArgs, const ToolChain &TC,
   const llvm::opt::ArgList &Args,
Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -668,6 +668,17 @@
   }
 }
 
+void tools::addOpenMPRuntimeLibraryPath(const ToolChain &TC,
+const ArgList &Args,
+ArgStringList &CmdArgs) {
+  // Default to clang lib / lib64 folder, i.e. the same location as device
+  // runtime.
+  SmallString<256> DefaultLibPath =
+  llvm::sys::path::parent_path(TC.getDriver().Dir);
+  llvm::sys::path::append(DefaultLibPath, Twine("lib") + CLANG_LIBDIR_SUFFIX);
+  CmdArgs.push_back(Args.MakeArgString("-L" + DefaultLibPath));
+}
+
 void tools::addArchSpecificRPath(const ToolChain &TC, const ArgList &Args,
  ArgStringList &CmdArgs) {
   // Enable -frtlib-add-rpath by default for the case of VE.
@@ -727,6 +738,7 @@
 
   if (RTKind == Driver::OMPRT_OMP)

[clang] 52fa1d1 - [Driver][AVR] Fix warn_drv_avr_stdlib_not_linked condition

[Fangrui Song via cfe-commits]

Author: Fangrui Song
Date: 2022-03-28T10:27:16-07:00
New Revision: 52fa1d1a02940ac46aac5b8d6f87ca9d6a6a97be

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

LOG: [Driver][AVR] Fix warn_drv_avr_stdlib_not_linked condition

Many options (-fsyntax-only, -E, -S, etc) skip the link action phase which the
existing condition does not account for.

Since the code no longer specifies OPT_c, I think a single RUN line about -c
not leading to a warning is sufficient. Adding one for all of -E,
-fsyntax-only, -S would be excessive.

Reviewed By: benshi001

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

Added: 


Modified: 
clang/lib/Driver/ToolChains/AVR.cpp
clang/lib/Driver/ToolChains/AVR.h
clang/test/Driver/avr-toolchain.c

Removed: 
clang/test/Driver/avr-link-no-mcu-specified.c



diff  --git a/clang/lib/Driver/ToolChains/AVR.cpp 
b/clang/lib/Driver/ToolChains/AVR.cpp
index 8688e06e78cc3..8ca8525e454b4 100644
--- a/clang/lib/Driver/ToolChains/AVR.cpp
+++ b/clang/lib/Driver/ToolChains/AVR.cpp
@@ -366,49 +366,19 @@ const StringRef PossibleAVRLibcLocations[] = {
 /// AVR Toolchain
 AVRToolChain::AVRToolChain(const Driver &D, const llvm::Triple &Triple,
const ArgList &Args)
-: Generic_ELF(D, Triple, Args), LinkStdlib(false) {
+: Generic_ELF(D, Triple, Args) {
   GCCInstallation.init(Triple, Args);
 
   // Only add default libraries if the user hasn't explicitly opted out.
   if (!Args.hasArg(options::OPT_nostdlib) &&
-  !Args.hasArg(options::OPT_nodefaultlibs) &&
-  !Args.hasArg(options::OPT_c /* does not apply when not linking */)) {
-std::string CPU = getCPUName(D, Args, Triple);
-
-if (CPU.empty()) {
-  // We cannot link any standard libraries without an MCU specified.
-  D.Diag(diag::warn_drv_avr_mcu_not_specified);
+  !Args.hasArg(options::OPT_nodefaultlibs)) {
+if (GCCInstallation.isValid()) {
+  GCCInstallPath = GCCInstallation.getInstallPath();
+  std::string GCCParentPath(GCCInstallation.getParentLibPath());
+  getProgramPaths().push_back(GCCParentPath + "/../bin");
 } else {
-  Optional FamilyName = GetMCUFamilyName(CPU);
-  Optional AVRLibcRoot = findAVRLibcInstallation();
-
-  if (!FamilyName.hasValue()) {
-// We do not have an entry for this CPU in the family
-// mapping table yet.
-D.Diag(diag::warn_drv_avr_family_linking_stdlibs_not_implemented)
-<< CPU;
-  } else if (!GCCInstallation.isValid()) {
-// No avr-gcc found and so no runtime linked.
-D.Diag(diag::warn_drv_avr_gcc_not_found);
-  } else if (!AVRLibcRoot.hasValue()) {
-// No avr-libc found and so no runtime linked.
-D.Diag(diag::warn_drv_avr_libc_not_found);
-  } else { // We have enough information to link stdlibs
-std::string GCCRoot(GCCInstallation.getInstallPath());
-std::string GCCParentPath(GCCInstallation.getParentLibPath());
-std::string LibcRoot = AVRLibcRoot.getValue();
-std::string SubPath = GetMCUSubPath(CPU);
-
-getProgramPaths().push_back(GCCParentPath + "/../bin");
-getFilePaths().push_back(LibcRoot + std::string("/lib/") + SubPath);
-getFilePaths().push_back(GCCRoot + std::string("/") + SubPath);
-
-LinkStdlib = true;
-  }
+  D.Diag(diag::warn_drv_avr_gcc_not_found);
 }
-
-if (!LinkStdlib)
-  D.Diag(diag::warn_drv_avr_stdlib_not_linked);
   }
 }
 
@@ -445,13 +415,14 @@ void AVRToolChain::addClangTargetOptions(
 }
 
 Tool *AVRToolChain::buildLinker() const {
-  return new tools::AVR::Linker(getTriple(), *this, LinkStdlib);
+  return new tools::AVR::Linker(getTriple(), *this);
 }
 
 void AVR::Linker::ConstructJob(Compilation &C, const JobAction &JA,
const InputInfo &Output,
const InputInfoList &Inputs, const ArgList 
&Args,
const char *LinkingOutput) const {
+  const auto &TC = static_cast(getToolChain());
   const Driver &D = getToolChain().getDriver();
 
   // Compute information about the target AVR.
@@ -473,6 +444,39 @@ void AVR::Linker::ConstructJob(Compilation &C, const 
JobAction &JA,
   Args.AddAllArgs(CmdArgs, options::OPT_L);
   getToolChain().AddFilePathLibArgs(Args, CmdArgs);
 
+  // Only add default libraries if the user hasn't explicitly opted out.
+  bool LinkStdlib = false;
+  if (!Args.hasArg(options::OPT_nostdlib) &&
+  !Args.hasArg(options::OPT_nodefaultlibs)) {
+if (CPU.empty()) {
+  // We cannot link any standard libraries without an MCU specified.
+  D.Diag(diag::warn_drv_avr_mcu_not_specified);
+} else {
+  Optional FamilyName = GetMCUFamilyName(CPU);
+  Optional AVRLibcRoot = TC.findAVRL

[PATCH] D122592: [OpenMP] Fix library path missing when using OpenMP

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert added a comment.

In D122592#3411812 , @MaskRay wrote:

> In D122592#3411795 , @ye-luo wrote:
>
>>> using the multiarch directory
>>
>> If we can cross compile libomp and libomptarget to the target system. We may 
>> have
>> lib/x86_64-unknown-linux-gnu/libomp.so
>> lib/aarch64-unknown-linux-gnu/libomp.so
>> Compile clang once but compile runtime library for multiple architectures.
>
> Yes, mostly for deambiguating cross compiling and multilib style -m32/-m64.
> In addition, I think all LLVM_ENABLE_RUNTIMES projects but openmp switched to 
> lib/$triple/xxx.so for runtime libraries, e.g. libc++.so, libc++abi.so, 
> libunwind.a.
> By switching openmp we will improve consistency. Downstream distributions can 
> use the same way to install runtime libraries, instead of doing different 
> things for openmp and non-openmp.

I'm all for it ;). Assuming @jhuber6 is not looking at that right now we need 
an issue to track this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122592

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

[PATCH] D122592: [OpenMP] Fix library path missing when using OpenMP

[Joseph Huber via Phabricator via cfe-commits]

jhuber6 added a comment.

In D122592#3411877 , @jdoerfert wrote:

> In D122592#3411812 , @MaskRay wrote:
>
>> In D122592#3411795 , @ye-luo wrote:
>>
 using the multiarch directory
>>>
>>> If we can cross compile libomp and libomptarget to the target system. We 
>>> may have
>>> lib/x86_64-unknown-linux-gnu/libomp.so
>>> lib/aarch64-unknown-linux-gnu/libomp.so
>>> Compile clang once but compile runtime library for multiple architectures.
>>
>> Yes, mostly for deambiguating cross compiling and multilib style -m32/-m64.
>> In addition, I think all LLVM_ENABLE_RUNTIMES projects but openmp switched 
>> to lib/$triple/xxx.so for runtime libraries, e.g. libc++.so, libc++abi.so, 
>> libunwind.a.
>> By switching openmp we will improve consistency. Downstream distributions 
>> can use the same way to install runtime libraries, instead of doing 
>> different things for openmp and non-openmp.
>
> I'm all for it ;). Assuming @jhuber6 is not looking at that right now we need 
> an issue to track this.

Sounds good, I won't look at it immediately but I could set up an issue to 
track the transition.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122592

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

[PATCH] D122524: [clang][AVR] Emit proper warnings

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

In D122524#3411872 , @benshi001 wrote:

> In D122524#3410542 , @MaskRay wrote:
>
>> I think it is excessive to add so many RUN lines. I do not understand much 
>> about AVR -mcpu. That said, I created D122553 
>>  for what I think should be done for the 
>> `-c/-S/-fsyntax-only` condition. More tests would just be excessive.
>>
>> You may adjust the patch to do the rest cleanups/fixes.
>
> I have made some changes based on your https://reviews.llvm.org/D122553, it 
> seems you have reverted. I suggest you recommit, since I have fixed the 
> failures of lacking avr-gcc.

I did not specify --sysroot so the new tests failed on systems without avr-gcc. 
Relanded.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122524

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

[PATCH] D122542: [flang][driver] Make --version and -version consistent with clang

[Emil Kieri via Phabricator via cfe-commits]

ekieri updated this revision to Diff 418643.
ekieri added a comment.

Address review comment.
Fix typo in summary.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122542

Files:
  clang/include/clang/Driver/Options.td
  flang/test/Driver/driver-help.f90
  flang/test/Driver/driver-version.f90


Index: flang/test/Driver/driver-version.f90
===
--- flang/test/Driver/driver-version.f90
+++ flang/test/Driver/driver-version.f90
@@ -2,15 +2,21 @@
 !---
 ! RUN LINES
 !---
-! RUN: %flang --version 2>&1 | FileCheck %s
+! RUN: %flang --version 2>&1 | FileCheck %s --check-prefix=VERSION
 ! RUN: not %flang --versions 2>&1 | FileCheck %s --check-prefix=ERROR
+! RUN: %flang_fc1 -version 2>&1 | FileCheck %s --check-prefix=VERSION-FC1
+! RUN: not %flang_fc1 --version 2>&1 | FileCheck %s --check-prefix=ERROR-FC1
 
 !---
 ! EXPECTED OUTPUT
 !---
-! CHECK: flang-new version
-! CHECK-NEXT: Target:
-! CHECK-NEXT: Thread model:
-! CHECK-NEXT: InstalledDir:
+! VERSION: flang-new version
+! VERSION-NEXT: Target:
+! VERSION-NEXT: Thread model:
+! VERSION-NEXT: InstalledDir:
 
 ! ERROR: flang-new: error: unsupported option '--versions'; did you mean 
'--version'?
+
+! VERSION-FC1: LLVM version
+
+! ERROR-FC1: error: unknown argument '--version'; did you mean '-version'?
Index: flang/test/Driver/driver-help.f90
===
--- flang/test/Driver/driver-help.f90
+++ flang/test/Driver/driver-help.f90
@@ -135,7 +135,7 @@
 ! HELP-FC1-NEXT: -test-io   Run the InputOuputTest action. Use for 
development and testing only.
 ! HELP-FC1-NEXT: -triple Specify target triple (e.g. 
i686-apple-darwin9)
 ! HELP-FC1-NEXT: -U  Undefine macro 
-! HELP-FC1-NEXT: --version  Print version information
+! HELP-FC1-NEXT: -version   Print the compiler version
 ! HELP-FC1-NEXT: -WEnable the specified warning
 
 !---
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -4205,7 +4205,7 @@
   HelpText<"Serialize compiler diagnostics to a file">;
 // We give --version different semantics from -version.
 def _version : Flag<["--"], "version">,
-  Flags<[CoreOption, FC1Option, FlangOption]>,
+  Flags<[CoreOption, FlangOption]>,
   HelpText<"Print version information">;
 def _signed_char : Flag<["--"], "signed-char">, Alias;
 def _std : Separate<["--"], "std">, Alias;
@@ -5749,11 +5749,16 @@
 // Language Options
 
//===--===//
 
-let Flags = [CC1Option, CC1AsOption, NoDriverOption] in {
+let Flags = [CC1Option, CC1AsOption, FC1Option, NoDriverOption] in {
 
 def version : Flag<["-"], "version">,
   HelpText<"Print the compiler version">,
   MarshallingInfoFlag>;
+
+} // let Flags = [CC1Option, CC1AsOption, FC1Option, NoDriverOption]
+
+let Flags = [CC1Option, CC1AsOption, NoDriverOption] in {
+
 def main_file_name : Separate<["-"], "main-file-name">,
   HelpText<"Main file name to use for debug info and source if missing">,
   MarshallingInfoString>;


Index: flang/test/Driver/driver-version.f90
===
--- flang/test/Driver/driver-version.f90
+++ flang/test/Driver/driver-version.f90
@@ -2,15 +2,21 @@
 !---
 ! RUN LINES
 !---
-! RUN: %flang --version 2>&1 | FileCheck %s
+! RUN: %flang --version 2>&1 | FileCheck %s --check-prefix=VERSION
 ! RUN: not %flang --versions 2>&1 | FileCheck %s --check-prefix=ERROR
+! RUN: %flang_fc1 -version 2>&1 | FileCheck %s --check-prefix=VERSION-FC1
+! RUN: not %flang_fc1 --version 2>&1 | FileCheck %s --check-prefix=ERROR-FC1
 
 !---
 ! EXPECTED OUTPUT
 !---
-! CHECK: flang-new version
-! CHECK-NEXT: Target:
-! CHECK-NEXT: Thread model:
-! CHECK-NEXT: InstalledDir:
+! VERSION: flang-new version
+! VERSION-NEXT: Target:
+! VERSION-NEXT: Thread model:
+! VERSION-NEXT: InstalledDir:
 
 ! ERROR: flang-new: error: unsupported option '--versions'; did you mean '--version'?
+
+! VERSION-FC1: LLVM version
+
+! ERROR-FC1: error: unknown argument '--version'; did you mean '-version'?
Index: flang/test/Driver/driver-help.f90
===
--- flang/test/Driver/driver-help.f90
+++ flang/test/Driver/driver-help.f90
@@ -135,7 +135,7 @@
 ! HELP-FC1-NEXT: -test-io   Run the InputOuputTest action. Use for development and testing only.
 ! HELP-FC1-NEXT: -triple Specify target triple (e.g. i686-apple-darwin9)
 ! HELP-FC1-NEXT: -U  Undefine macro 
-! HELP-FC1-NEXT: --version  Print version information

[PATCH] D122425: Pass -disable-llvm-passes to avoid running llvm passes

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak added a comment.

Yes, this test was susceptible to changes made to llvm passes and the pass 
pipeline.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122425

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