[PATCH] D57497: [RISCV] Passing small data limitation value to RISCV backend

[James Clarke via Phabricator via cfe-commits]

jrtc27 added a comment.

I believe my previous comments have indeed been addressed.




Comment at: clang/docs/ClangCommandLineReference.rst:2958
+
+Put global and static data smaller than the limitation into a special section 
(RISCV only)
+

lenary wrote:
> "(RISC-V only)" please
This seems like needless noise; it's already in the RISCV section, we don't 
need to say it's RISC-V only in the body too. Other architectures are 
inconsistent about this but I see it as a waste of time?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D57497



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

[PATCH] D74704: Support -fuse-ld=lld for riscv

[James Clarke via Phabricator via cfe-commits]

jrtc27 added a comment.

In D74704#1878921 , @lenary wrote:

> I am worried about the interaction between `-fuse-ld=lld` and linker 
> relaxation (which is not supported by LLD, as I understand it)
>
> 1. clang could ignore `-fuse-ld=lld` when linker relaxation is enabled
> 2. clang could ignore (disable) linker relaxation if `-fuse-ld=lld` is used
>
>   At the very least, a warning of some kind should be emitted if linker 
> relaxation is combined with `-fuse-ld=lld`.


This isn't a new problem. The Linux and FreeBSD toolchains already support 
-fuse-ld=lld properly, it's just the bare metal one that didn't. People also 
generally don't include -fuse-ld=lld in CFLAGS, only LDFLAGS, and there's no 
need to include -mno-relax in LDFLAGS, so I suspect you wouldn't catch many 
issues. These days (https://reviews.llvm.org/D71820), LLD will give you an 
informative error (rather than silently mis-linking) if you forget -mno-relax.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74704



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

[PATCH] D69590: [RISCV] Fix ILP32D lowering for double+double/double+int return types

[James Clarke via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG3d6c492d7a98: [RISCV] Fix ILP32D lowering for 
double+double/double+int return types (authored by jrtc27).

Changed prior to commit:
  https://reviews.llvm.org/D69590?vs=226981&id=237912#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69590

Files:
  clang/lib/CodeGen/TargetInfo.cpp
  clang/test/CodeGen/riscv32-ilp32d-abi.c


Index: clang/test/CodeGen/riscv32-ilp32d-abi.c
===
--- clang/test/CodeGen/riscv32-ilp32d-abi.c
+++ clang/test/CodeGen/riscv32-ilp32d-abi.c
@@ -280,3 +280,27 @@
 union double_u f_ret_double_u() {
   return (union double_u){1.0};
 }
+
+// Test that we don't incorrectly think double+int/double+double structs will
+// be returned indirectly and thus have an off-by-one error for the number of
+// GPRs available (this is an edge case when structs > 2*XLEN are still
+// returned in registers). This includes complex doubles, which are treated as
+// double+double structs by the ABI.
+
+// CHECK: define { double, i32 } 
@f_ret_double_int32_s_double_int32_s_just_sufficient_gprs(i32 %a, i32 %b, i32 
%c, i32 %d, i32 %e, i32 %f, i32 %g, double %0, i32 %1)
+struct double_int32_s f_ret_double_int32_s_double_int32_s_just_sufficient_gprs(
+int a, int b, int c, int d, int e, int f, int g, struct double_int32_s h) {
+  return (struct double_int32_s){1.0, 2};
+}
+
+// CHECK: define { double, double } 
@f_ret_double_double_s_double_int32_s_just_sufficient_gprs(i32 %a, i32 %b, i32 
%c, i32 %d, i32 %e, i32 %f, i32 %g, double %0, i32 %1)
+struct double_double_s 
f_ret_double_double_s_double_int32_s_just_sufficient_gprs(
+int a, int b, int c, int d, int e, int f, int g, struct double_int32_s h) {
+  return (struct double_double_s){1.0, 2.0};
+}
+
+// CHECK: define { double, double } 
@f_ret_doublecomplex_double_int32_s_just_sufficient_gprs(i32 %a, i32 %b, i32 
%c, i32 %d, i32 %e, i32 %f, i32 %g, double %0, i32 %1)
+double __complex__ f_ret_doublecomplex_double_int32_s_just_sufficient_gprs(
+int a, int b, int c, int d, int e, int f, int g, struct double_int32_s h) {
+  return 1.0;
+}
Index: clang/lib/CodeGen/TargetInfo.cpp
===
--- clang/lib/CodeGen/TargetInfo.cpp
+++ clang/lib/CodeGen/TargetInfo.cpp
@@ -9377,11 +9377,21 @@
 FI.getReturnInfo() = classifyReturnType(RetTy);
 
   // IsRetIndirect is true if classifyArgumentType indicated the value should
-  // be passed indirect or if the type size is greater than 2*xlen. e.g. fp128
-  // is passed direct in LLVM IR, relying on the backend lowering code to
-  // rewrite the argument list and pass indirectly on RV32.
-  bool IsRetIndirect = FI.getReturnInfo().getKind() == ABIArgInfo::Indirect ||
-   getContext().getTypeSize(RetTy) > (2 * XLen);
+  // be passed indirect, or if the type size is a scalar greater than 2*XLen
+  // and not a complex type with elements <= FLen. e.g. fp128 is passed direct
+  // in LLVM IR, relying on the backend lowering code to rewrite the argument
+  // list and pass indirectly on RV32.
+  bool IsRetIndirect = FI.getReturnInfo().getKind() == ABIArgInfo::Indirect;
+  if (!IsRetIndirect && RetTy->isScalarType() &&
+  getContext().getTypeSize(RetTy) > (2 * XLen)) {
+if (RetTy->isComplexType() && FLen) {
+  QualType EltTy = RetTy->getAs()->getElementType();
+  IsRetIndirect = getContext().getTypeSize(EltTy) > FLen;
+} else {
+  // This is a normal scalar > 2*XLen, such as fp128 on RV32.
+  IsRetIndirect = true;
+}
+  }
 
   // We must track the number of GPRs used in order to conform to the RISC-V
   // ABI, as integer scalars passed in registers should have signext/zeroext


Index: clang/test/CodeGen/riscv32-ilp32d-abi.c
===
--- clang/test/CodeGen/riscv32-ilp32d-abi.c
+++ clang/test/CodeGen/riscv32-ilp32d-abi.c
@@ -280,3 +280,27 @@
 union double_u f_ret_double_u() {
   return (union double_u){1.0};
 }
+
+// Test that we don't incorrectly think double+int/double+double structs will
+// be returned indirectly and thus have an off-by-one error for the number of
+// GPRs available (this is an edge case when structs > 2*XLEN are still
+// returned in registers). This includes complex doubles, which are treated as
+// double+double structs by the ABI.
+
+// CHECK: define { double, i32 } @f_ret_double_int32_s_double_int32_s_just_sufficient_gprs(i32 %a, i32 %b, i32 %c, i32 %d, i32 %e, i32 %f, i32 %g, double %0, i32 %1)
+struct double_int32_s f_ret_double_int32_s_double_int32_s_just_sufficient_gprs(
+int a, int b, int c, int d, int e, int f, int g, struct double_int32_s h) {
+  return (struct double_int32_s){1.0, 2};
+}
+
+// CHECK: define { double, double } @f_ret_double_double_s_double_int32_s_just_sufficient

[PATCH] D71499: Add builtins for aligning and checking alignment of pointers and integers

[James Clarke via Phabricator via cfe-commits]

jrtc27 added inline comments.



Comment at: clang/docs/LanguageExtensions.rst:2844
+The builtins ``__builtin_align_up``, ``__builtin_align_down``, return their
+first argument aligned up/down to the next multiple of the second argument.
+The builtin ``__builtin_is_aligned`` returns whether the first argument is

This should clearly state whether an already-aligned argument is left 
unmodified or incremented/decremented by the alignment amount. As it stands it 
reads more like such arguments will be modified, but the implementation is to 
round, ie the former.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71499



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

[PATCH] D72056: [RISCV] Generate PIC address sequence for medany -fno-pic

[James Clarke via Phabricator via cfe-commits]

jrtc27 added inline comments.



Comment at: llvm/lib/Target/TargetMachine.cpp:192
   return false;
+// RISC-V non-small code models prefer avoiding copy relocations.
+if (TT.isRISCV() && getCodeModel() != CodeModel::Small)

MaskRay wrote:
> jrtc27 wrote:
> > Are we sure we want to do this and take the performance hit over GCC due to 
> > the extra level of indirection on every single extern global access? If 
> > this is the solution to take for extern weak, I think we should limit it to 
> > just that and have a separate discussion about non-extern-weak.
> I'd like to know more about `-mcmodel=medany -fno-pic`'s use cases. I guess 
> @bsdjhb's FreeBSD kernel use case can probably be met by `-mcmodel=medany 
> -fpie`.
> 
> I think there is no precedent which differs extern-strong and extern-weak. If 
> GOT relaxation works, we can make this unconditional.
> 
>   // RISC-V prefer avoiding copy relocations.
>   if (TT.isRISCV()
> return false;
It can, but that implies GOT accesses for anything extern, which will hurt 
kernel performance.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72056



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

[PATCH] D72056: [RISCV] Generate PIC address sequence for medany -fno-pic

[James Clarke via Phabricator via cfe-commits]

jrtc27 added a comment.

I am still of the view that we should support rewriting the instruction stream 
in the linker when necessary like BFD does. We need to do this to be able to 
link in GCC-compiled code, including libraries. It is a very simple thing to do 
with a patch available that provides consistency between the GNU world and the 
LLVM world.




Comment at: llvm/lib/Target/RISCV/RISCVExpandPseudoInsts.cpp:680
+  if (MF->getTarget().isPositionIndependent() ||
+  MF->getTarget().getCodeModel() != CodeModel::Small) {
 const auto &STI = MF->getSubtarget();

This is confusing and differs from when you invoke the assembler manually, even 
via the Clang driver with the right code model specified. Any `la`s there will 
be assembled as `AUIPC`/`ADDI`. I am of the view it was a mistake to make 
`la`'s behaviour conditional on PICness and it should have always used the GOT, 
but this is what we have.



Comment at: llvm/lib/Target/TargetMachine.cpp:192
   return false;
+// RISC-V non-small code models prefer avoiding copy relocations.
+if (TT.isRISCV() && getCodeModel() != CodeModel::Small)

Are we sure we want to do this and take the performance hit over GCC due to the 
extra level of indirection on every single extern global access? If this is the 
solution to take for extern weak, I think we should limit it to just that and 
have a separate discussion about non-extern-weak.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72056



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

[PATCH] D62686: [RISCV] Add support for save/restore of callee-saved registers via libcalls

[James Clarke via Phabricator via cfe-commits]

jrtc27 added inline comments.



Comment at: llvm/lib/Target/RISCV/RISCVFrameLowering.cpp:668
+.setMIFlag(MachineInstr::FrameSetup);
+
+// Add registers spilled in libcall as liveins.

lewis-revill wrote:
> shiva0217 wrote:
> > GCC will generate stack adjustment and GPR callee saved CFIs for the save 
> > libcalls. Should we do the same?
> I'm not familiar with the use of the CFI offset stuff, though just to be 
> sure, you're saying that in addition to the manually-added `.cfi_offset`s in 
> the libcalls themselves we would want to add them in our frame too?
Yes. The CFI use is completely ignorant of any control flow. It goes to the 
start of the function, and walks forward, executing every `.cfi_*` directive 
until reaching the current point in the code. Since the libcalls (and thus 
their CFI directives) are not inline in the function, they are completely 
invisible to the CFI machinery. Thus, their side-effects must be duplicated 
straight after the calls to them.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62686



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

[PATCH] D69590: [RISCV] Fix ILP32D lowering for double+double/double+int return types

[James Clarke via Phabricator via cfe-commits]

jrtc27 created this revision.
jrtc27 added a reviewer: asb.
Herald added subscribers: cfe-commits, sameer.abuasal, pzheng, s.egerton, 
lenary, Jim, benna, psnobl, jocewei, PkmX, rkruppe, the_o, brucehoult, 
MartinMosbeck, rogfer01, edward-jones, zzheng, shiva0217, kito-cheng, niosHD, 
sabuasal, apazos, simoncook, johnrusso, rbar.
Herald added a project: clang.

Previously, since these aggregates are > 2*XLen, Clang would think they
were being returned indirectly and thus would decrease the number of
available GPRs available by 1. For long argument lists this could lead
to a struct argument incorrectly being passed indirectly.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D69590

Files:
  clang/lib/CodeGen/TargetInfo.cpp
  clang/test/CodeGen/riscv32-ilp32d-abi.c


Index: clang/test/CodeGen/riscv32-ilp32d-abi.c
===
--- clang/test/CodeGen/riscv32-ilp32d-abi.c
+++ clang/test/CodeGen/riscv32-ilp32d-abi.c
@@ -280,3 +280,27 @@
 union double_u f_ret_double_u() {
   return (union double_u){1.0};
 }
+
+// Test that we don't incorrectly think double+int/double+double structs will
+// be returned indirectly and thus have an off-by-one error for the number of
+// GPRs available (this is an edge case when structs > 2*XLEN are still
+// returned in registers). This include complex doubles, which are treated as
+// double+double structs by the ABI.
+
+// CHECK: define { double, i32 } 
@f_ret_double_int32_s_double_int32_s_just_sufficient_gprs(i32 %a, i32 %b, i32 
%c, i32 %d, i32 %e, i32 %f, i32 %g, double %0, i32 %1)
+struct double_int32_s f_ret_double_int32_s_double_int32_s_just_sufficient_gprs(
+int a, int b, int c, int d, int e, int f, int g, struct double_int32_s h) {
+  return (struct double_int32_s){1.0, 2};
+}
+
+// CHECK: define { double, double } 
@f_ret_double_double_s_double_int32_s_just_sufficient_gprs(i32 %a, i32 %b, i32 
%c, i32 %d, i32 %e, i32 %f, i32 %g, double %0, i32 %1)
+struct double_double_s 
f_ret_double_double_s_double_int32_s_just_sufficient_gprs(
+int a, int b, int c, int d, int e, int f, int g, struct double_int32_s h) {
+  return (struct double_double_s){1.0, 2.0};
+}
+
+// CHECK: define { double, double } 
@f_ret_doublecomplex_double_int32_s_just_sufficient_gprs(i32 %a, i32 %b, i32 
%c, i32 %d, i32 %e, i32 %f, i32 %g, double %0, i32 %1)
+double __complex__ f_ret_doublecomplex_double_int32_s_just_sufficient_gprs(
+int a, int b, int c, int d, int e, int f, int g, struct double_int32_s h) {
+  return 1.0;
+}
Index: clang/lib/CodeGen/TargetInfo.cpp
===
--- clang/lib/CodeGen/TargetInfo.cpp
+++ clang/lib/CodeGen/TargetInfo.cpp
@@ -9291,11 +9291,21 @@
 FI.getReturnInfo() = classifyReturnType(RetTy);
 
   // IsRetIndirect is true if classifyArgumentType indicated the value should
-  // be passed indirect or if the type size is greater than 2*xlen. e.g. fp128
-  // is passed direct in LLVM IR, relying on the backend lowering code to
-  // rewrite the argument list and pass indirectly on RV32.
-  bool IsRetIndirect = FI.getReturnInfo().getKind() == ABIArgInfo::Indirect ||
-   getContext().getTypeSize(RetTy) > (2 * XLen);
+  // be passed indirect, or if the type size is a scalar greater than 2*XLen
+  // and not a complex type with elements <= FLen. e.g. fp128 is passed direct
+  // in LLVM IR, relying on the backend lowering code to rewrite the argument
+  // list and pass indirectly on RV32.
+  bool IsRetIndirect = FI.getReturnInfo().getKind() == ABIArgInfo::Indirect;
+  if (!IsRetIndirect && RetTy->isScalarType() &&
+  getContext().getTypeSize(RetTy) > (2 * XLen)) {
+if (RetTy->isComplexType() && FLen) {
+  QualType EltTy = RetTy->getAs()->getElementType();
+  IsRetIndirect = getContext().getTypeSize(EltTy) > FLen;
+} else {
+  // This is a normal scalar > 2*XLen, such as fp128 on RV32.
+  IsRetIndirect = true;
+}
+  }
 
   // We must track the number of GPRs used in order to conform to the RISC-V
   // ABI, as integer scalars passed in registers should have signext/zeroext


Index: clang/test/CodeGen/riscv32-ilp32d-abi.c
===
--- clang/test/CodeGen/riscv32-ilp32d-abi.c
+++ clang/test/CodeGen/riscv32-ilp32d-abi.c
@@ -280,3 +280,27 @@
 union double_u f_ret_double_u() {
   return (union double_u){1.0};
 }
+
+// Test that we don't incorrectly think double+int/double+double structs will
+// be returned indirectly and thus have an off-by-one error for the number of
+// GPRs available (this is an edge case when structs > 2*XLEN are still
+// returned in registers). This include complex doubles, which are treated as
+// double+double structs by the ABI.
+
+// CHECK: define { double, i32 } @f_ret_double_int32_s_double_int32_s_just_sufficient_gprs(i32 %a, i32 %b, i32 %c, i32 %d, i32 %e, i32 %f, i32 %g, double %0, i32 %1)
+struct double_in

[PATCH] D54214: [RISCV] Set triple based on -march flag

[James Clarke via Phabricator via cfe-commits]

jrtc27 added a comment.

In D54214#1718043 , @simoncook wrote:

> Ping, before I rebased this did anyone have any other thoughts on flag 
> precedence?


The precedence seems sensible; it's what MIPS is doing for -mabi vs -target 
just above.




Comment at: clang/lib/Driver/Driver.cpp:541
+  // If target is RISC-V adjust the target triple according to
+  // provided architecture name
+  A = Args.getLastArg(options::OPT_march_EQ);

Missing a full-stop here


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D54214



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

[PATCH] D58531: [clang] Specify type of pthread_create builtin

[James Clarke via Phabricator via cfe-commits]

jrtc27 marked an inline comment as done.
jrtc27 added a comment.

In D58531#1662466 , @jdoerfert wrote:

> I like the patch and I think it is fine.
>
> Small nits: 
>  Could we have a test for " we can detect dodgy pthread_create declarations" 
> and maybe `pthread[_attr]_t`?
>  There is an unresolved comment by @shrines.
>  @probinson: "it seems straightforward enough although clearly needs 
> clang-format-diff run over it."
>
> I'll accept this assuming the above points are easy to fix and given that no 
> one expressed concerns but only positive comments were made.


I believe I have addressed everything, with the possible exception of:

> Could we have a test for " we can detect dodgy pthread_create declarations" 
> and maybe `pthread[_attr]_t`?

Is that not what I had already added to 
`clang/test/Sema/implicit-builtin-decl.c`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D58531



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

[PATCH] D58531: [clang] Specify type of pthread_create builtin

[James Clarke via Phabricator via cfe-commits]

jrtc27 updated this revision to Diff 223871.
jrtc27 added a comment.

Rebased, added explicit no-warning test, clang-format'ed, updated assertion 
message.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D58531

Files:
  clang/include/clang/AST/ASTContext.h
  clang/include/clang/Basic/Builtins.def
  clang/include/clang/Serialization/ASTBitCodes.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTWriter.cpp
  clang/test/Sema/implicit-builtin-decl.c
  clang/test/Sema/implicit-builtin-redecl.c

Index: clang/test/Sema/implicit-builtin-redecl.c
===
--- clang/test/Sema/implicit-builtin-redecl.c
+++ clang/test/Sema/implicit-builtin-redecl.c
@@ -24,3 +24,9 @@
 }
 
 typedef int rindex;
+
+// PR40692
+typedef void pthread_t;
+typedef void pthread_attr_t;
+int pthread_create(pthread_t *, const pthread_attr_t *, // no-warning
+   void *(*)(void *), void *);
Index: clang/test/Sema/implicit-builtin-decl.c
===
--- clang/test/Sema/implicit-builtin-decl.c
+++ clang/test/Sema/implicit-builtin-decl.c
@@ -68,4 +68,4 @@
 // CHECK: ReturnsTwiceAttr {{.*}} <{{.*}}> Implicit
 
 // PR40692
-void pthread_create(); // no warning expected
+void pthread_create(); // expected-warning{{declaration of built-in function 'pthread_create' requires inclusion of the header }}
Index: clang/lib/Serialization/ASTWriter.cpp
===
--- clang/lib/Serialization/ASTWriter.cpp
+++ clang/lib/Serialization/ASTWriter.cpp
@@ -4966,6 +4966,8 @@
   AddTypeRef(Context.ObjCClassRedefinitionType, SpecialTypes);
   AddTypeRef(Context.ObjCSelRedefinitionType, SpecialTypes);
   AddTypeRef(Context.getucontext_tType(), SpecialTypes);
+  AddTypeRef(Context.getpthread_tType(), SpecialTypes);
+  AddTypeRef(Context.getpthread_attr_tType(), SpecialTypes);
 
   if (Chain) {
 // Write the mapping information describing our module dependencies and how
Index: clang/lib/Serialization/ASTReader.cpp
===
--- clang/lib/Serialization/ASTReader.cpp
+++ clang/lib/Serialization/ASTReader.cpp
@@ -4847,6 +4847,43 @@
 }
   }
 }
+
+if (unsigned Pthread_t = SpecialTypes[SPECIAL_TYPE_PTHREAD_T]) {
+  QualType Pthread_tType = GetType(Pthread_t);
+  if (Pthread_tType.isNull()) {
+Error("pthread_t type is NULL");
+return;
+  }
+
+  if (!Context.pthread_tDecl) {
+if (const TypedefType *Typedef = Pthread_tType->getAs())
+  Context.setpthread_tDecl(Typedef->getDecl());
+else {
+  const TagType *Tag = Pthread_tType->getAs();
+  assert(Tag && "Invalid pthread_t type in AST file");
+  Context.setpthread_tDecl(Tag->getDecl());
+}
+  }
+}
+
+if (unsigned Pthread_attr_t = SpecialTypes[SPECIAL_TYPE_PTHREAD_ATTR_T]) {
+  QualType Pthread_attr_tType = GetType(Pthread_attr_t);
+  if (Pthread_attr_tType.isNull()) {
+Error("pthread_attr_t type is NULL");
+return;
+  }
+
+  if (!Context.pthread_attr_tDecl) {
+if (const TypedefType *Typedef =
+Pthread_attr_tType->getAs())
+  Context.setpthread_attr_tDecl(Typedef->getDecl());
+else {
+  const TagType *Tag = Pthread_attr_tType->getAs();
+  assert(Tag && "Invalid pthread_attr_t type in AST file");
+  Context.setpthread_attr_tDecl(Tag->getDecl());
+}
+  }
+}
   }
 
   ReadPragmaDiagnosticMappings(Context.getDiagnostics());
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -1968,6 +1968,8 @@
 return "setjmp.h";
   case ASTContext::GE_Missing_ucontext:
 return "ucontext.h";
+  case ASTContext::GE_Missing_pthread:
+return "pthread.h";
   }
   llvm_unreachable("unhandled error kind");
 }
@@ -5970,6 +5972,10 @@
 Context.setsigjmp_bufDecl(NewTD);
   else if (II->isStr("ucontext_t"))
 Context.setucontext_tDecl(NewTD);
+  else if (II->isStr("pthread_t"))
+Context.setpthread_tDecl(NewTD);
+  else if (II->isStr("pthread_attr_t"))
+Context.setpthread_attr_tDecl(NewTD);
 }
 
   return NewTD;
Index: clang/lib/AST/ASTContext.cpp
===
--- clang/lib/AST/ASTContext.cpp
+++ clang/lib/AST/ASTContext.cpp
@@ -9383,6 +9383,11 @@
 //  Builtin Type Computation
 //===--===//
 
+static QualType DecodeFunctionTypeFromStr(
+const char *&TypeStr, bool IsNested, bool IsNoReturn, bool IsNoThrow,
+   

[PATCH] D68526: [Diagnostics] Silence -Wsizeof-array-div for character buffers

[James Clarke via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG67f542aba72f: [Diagnostics] Silence -Wsizeof-array-div for 
character buffers (authored by jrtc27).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68526

Files:
  clang/lib/Sema/SemaExpr.cpp
  clang/test/Sema/div-sizeof-array.cpp


Index: clang/test/Sema/div-sizeof-array.cpp
===
--- clang/test/Sema/div-sizeof-array.cpp
+++ clang/test/Sema/div-sizeof-array.cpp
@@ -25,6 +25,8 @@
   int a10 = sizeof(arr3) / sizeof(char);
   int a11 = sizeof(arr2) / (sizeof(unsigned));
   int a12 = sizeof(arr) / (sizeof(short));
+  int a13 = sizeof(arr3) / sizeof(p);
+  int a14 = sizeof(arr3) / sizeof(int);
 
   int arr4[10][12];
   int b1 = sizeof(arr4) / sizeof(arr2[12]);
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -9197,6 +9197,7 @@
 QualType ArrayElemTy = ArrayTy->getElementType();
 if (ArrayElemTy != S.Context.getBaseElementType(ArrayTy) ||
 ArrayElemTy->isDependentType() || RHSTy->isDependentType() ||
+ArrayElemTy->isCharType() ||
 S.Context.getTypeSize(ArrayElemTy) == S.Context.getTypeSize(RHSTy))
   return;
 S.Diag(Loc, diag::warn_division_sizeof_array)


Index: clang/test/Sema/div-sizeof-array.cpp
===
--- clang/test/Sema/div-sizeof-array.cpp
+++ clang/test/Sema/div-sizeof-array.cpp
@@ -25,6 +25,8 @@
   int a10 = sizeof(arr3) / sizeof(char);
   int a11 = sizeof(arr2) / (sizeof(unsigned));
   int a12 = sizeof(arr) / (sizeof(short));
+  int a13 = sizeof(arr3) / sizeof(p);
+  int a14 = sizeof(arr3) / sizeof(int);
 
   int arr4[10][12];
   int b1 = sizeof(arr4) / sizeof(arr2[12]);
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -9197,6 +9197,7 @@
 QualType ArrayElemTy = ArrayTy->getElementType();
 if (ArrayElemTy != S.Context.getBaseElementType(ArrayTy) ||
 ArrayElemTy->isDependentType() || RHSTy->isDependentType() ||
+ArrayElemTy->isCharType() ||
 S.Context.getTypeSize(ArrayElemTy) == S.Context.getTypeSize(RHSTy))
   return;
 S.Diag(Loc, diag::warn_division_sizeof_array)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D68526: [Diagnostics] Silence -Wsizeof-array-div for character buffers

[James Clarke via Phabricator via cfe-commits]

jrtc27 updated this revision to Diff 223823.
jrtc27 added a comment.

Rebased


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68526

Files:
  clang/lib/Sema/SemaExpr.cpp
  clang/test/Sema/div-sizeof-array.cpp


Index: clang/test/Sema/div-sizeof-array.cpp
===
--- clang/test/Sema/div-sizeof-array.cpp
+++ clang/test/Sema/div-sizeof-array.cpp
@@ -25,6 +25,8 @@
   int a10 = sizeof(arr3) / sizeof(char);
   int a11 = sizeof(arr2) / (sizeof(unsigned));
   int a12 = sizeof(arr) / (sizeof(short));
+  int a13 = sizeof(arr3) / sizeof(p);
+  int a14 = sizeof(arr3) / sizeof(int);
 
   int arr4[10][12];
   int b1 = sizeof(arr4) / sizeof(arr2[12]);
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -9197,6 +9197,7 @@
 QualType ArrayElemTy = ArrayTy->getElementType();
 if (ArrayElemTy != S.Context.getBaseElementType(ArrayTy) ||
 ArrayElemTy->isDependentType() || RHSTy->isDependentType() ||
+ArrayElemTy->isCharType() ||
 S.Context.getTypeSize(ArrayElemTy) == S.Context.getTypeSize(RHSTy))
   return;
 S.Diag(Loc, diag::warn_division_sizeof_array)


Index: clang/test/Sema/div-sizeof-array.cpp
===
--- clang/test/Sema/div-sizeof-array.cpp
+++ clang/test/Sema/div-sizeof-array.cpp
@@ -25,6 +25,8 @@
   int a10 = sizeof(arr3) / sizeof(char);
   int a11 = sizeof(arr2) / (sizeof(unsigned));
   int a12 = sizeof(arr) / (sizeof(short));
+  int a13 = sizeof(arr3) / sizeof(p);
+  int a14 = sizeof(arr3) / sizeof(int);
 
   int arr4[10][12];
   int b1 = sizeof(arr4) / sizeof(arr2[12]);
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -9197,6 +9197,7 @@
 QualType ArrayElemTy = ArrayTy->getElementType();
 if (ArrayElemTy != S.Context.getBaseElementType(ArrayTy) ||
 ArrayElemTy->isDependentType() || RHSTy->isDependentType() ||
+ArrayElemTy->isCharType() ||
 S.Context.getTypeSize(ArrayElemTy) == S.Context.getTypeSize(RHSTy))
   return;
 S.Diag(Loc, diag::warn_division_sizeof_array)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D68368: [ItaniumMangle] Fix mangling of GNU __null in an expression to match GCC

[James Clarke via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL374013: [ItaniumMangle] Fix mangling of GNU __null in an 
expression to match GCC (authored by jrtc27, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D68368?vs=222949&id=223783#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D68368

Files:
  cfe/trunk/lib/AST/ItaniumMangle.cpp
  cfe/trunk/test/CodeGenCXX/mangle-exprs.cpp


Index: cfe/trunk/lib/AST/ItaniumMangle.cpp
===
--- cfe/trunk/lib/AST/ItaniumMangle.cpp
+++ cfe/trunk/lib/AST/ItaniumMangle.cpp
@@ -4273,8 +4273,11 @@
   }
 
   case Expr::GNUNullExprClass:
-// FIXME: should this really be mangled the same as nullptr?
-// fallthrough
+// Mangle as if an integer literal 0.
+Out << 'L';
+mangleType(E->getType());
+Out << "0E";
+break;
 
   case Expr::CXXNullPtrLiteralExprClass: {
 Out << "LDnE";
Index: cfe/trunk/test/CodeGenCXX/mangle-exprs.cpp
===
--- cfe/trunk/test/CodeGenCXX/mangle-exprs.cpp
+++ cfe/trunk/test/CodeGenCXX/mangle-exprs.cpp
@@ -373,3 +373,19 @@
   template void f(decltype(T{.a.b[3][1 ... 4] = 9}) x) {}
   void use_f(A a) { f(a); }
 }
+
+namespace null {
+  template 
+  void cpp_nullptr(typename enable_if::type* = 0) {
+  }
+
+  template 
+  void gnu_null(typename enable_if::type* = 0) {
+  }
+
+  // CHECK-LABEL: define {{.*}} 
@_ZN4null11cpp_nullptrILDn0EEEvPN9enable_ifIXeqT_LDnEEvE4typeE
+  template void cpp_nullptr(void *);
+
+  // CHECK-LABEL: define {{.*}} 
@_ZN4null8gnu_nullILPv0EEEvPN9enable_ifIXeqT_Ll0EEvE4typeE
+  template void gnu_null(void *);
+}


Index: cfe/trunk/lib/AST/ItaniumMangle.cpp
===
--- cfe/trunk/lib/AST/ItaniumMangle.cpp
+++ cfe/trunk/lib/AST/ItaniumMangle.cpp
@@ -4273,8 +4273,11 @@
   }
 
   case Expr::GNUNullExprClass:
-// FIXME: should this really be mangled the same as nullptr?
-// fallthrough
+// Mangle as if an integer literal 0.
+Out << 'L';
+mangleType(E->getType());
+Out << "0E";
+break;
 
   case Expr::CXXNullPtrLiteralExprClass: {
 Out << "LDnE";
Index: cfe/trunk/test/CodeGenCXX/mangle-exprs.cpp
===
--- cfe/trunk/test/CodeGenCXX/mangle-exprs.cpp
+++ cfe/trunk/test/CodeGenCXX/mangle-exprs.cpp
@@ -373,3 +373,19 @@
   template void f(decltype(T{.a.b[3][1 ... 4] = 9}) x) {}
   void use_f(A a) { f(a); }
 }
+
+namespace null {
+  template 
+  void cpp_nullptr(typename enable_if::type* = 0) {
+  }
+
+  template 
+  void gnu_null(typename enable_if::type* = 0) {
+  }
+
+  // CHECK-LABEL: define {{.*}} @_ZN4null11cpp_nullptrILDn0EEEvPN9enable_ifIXeqT_LDnEEvE4typeE
+  template void cpp_nullptr(void *);
+
+  // CHECK-LABEL: define {{.*}} @_ZN4null8gnu_nullILPv0EEEvPN9enable_ifIXeqT_Ll0EEvE4typeE
+  template void gnu_null(void *);
+}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D68526: [Diagnostics] Silence -Wsizeof-array-div for character buffers

[James Clarke via Phabricator via cfe-commits]

jrtc27 added a comment.

In D68526#1696587 , @lebedev.ri wrote:

> There's not a single word in the patch's description.


If I were to add a description, it would be something like:

  Character buffers are sometimes used to represent a pool of memory that
  contains non-character objects, due to them being synonymous with a stream of
  bytes on almost all modern architectures. Often, when interacting with 
hardware
  devices, byte buffers are therefore used as an intermediary and so we can end
  Character buffers are sometimes used to represent a pool of memory that
  contains non-character objects, due to them being synonymous with a stream of
  bytes on almost all modern architectures. Often, when interacting with 
hardware
  devices, byte buffers are therefore used as an intermediary and so we can end
  up generating lots of false-positives.
  
  Moreover, due to the ability of character pointers to alias non-character
  pointers, the strict aliasing violations that would generally be implied by 
the
  calculations caught by the warning (if the calculation itself is in fact
  correct) do not apply here, and so although the length calculation may be
  wrong, that is the only possible issue.

But it seems that this first premise is actually wrong; at least in FreeBSD, 
and using a quick grep through Linux for obvious names (`sizeof.*buf) /`, 
`sizeof.*buffer) /` and `sizeof.*data) /`) I couldn't find one instance of an 
array where the types didn't match, but that was for `u64` vs `u32` so isn't 
even covered by this. Maybe it was true in the past, but these days people seem 
to be diligent about using the right types, with proper unions when needed.

The aliasing point still applies (in that this case is slightly less 
dangerous), but given it draws attention to code that may be able to be written 
more nicely with better types, I'd be inclined to abandon this. Unless people 
come screaming that we're spewing too many warnings for their code, that is, of 
course.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68526



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

[PATCH] D68526: [Diagnostics] Silence -Wsizeof-array-div for character buffers

[James Clarke via Phabricator via cfe-commits]

jrtc27 added a comment.

Having said that, the one warning this raised in FreeBSD (at least for our 
configuration) was a completely unnecessary use of a character buffer and could 
be cleaned up, so maybe this case is a useful one to catch. Does anyone else 
have an opinion on this?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68526



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

[PATCH] D68526: [Diagnostics] Silence -Wsizeof-array-div for character buffers

[James Clarke via Phabricator via cfe-commits]

jrtc27 created this revision.
jrtc27 added reviewers: rsmith, xbolva00.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D68526

Files:
  clang/lib/Sema/SemaExpr.cpp
  clang/test/Sema/div-sizeof-array.cpp


Index: clang/test/Sema/div-sizeof-array.cpp
===
--- clang/test/Sema/div-sizeof-array.cpp
+++ clang/test/Sema/div-sizeof-array.cpp
@@ -21,6 +21,8 @@
   int a9 = sizeof(arr) / sizeof(unsigned int);
   const char arr3[2] = "A";
   int a10 = sizeof(arr3) / sizeof(char);
+  int a11 = sizeof(arr3) / sizeof(p);
+  int a12 = sizeof(arr3) / sizeof(int);
 
   int arr4[10][12]; // expected-note 3 {{array 'arr4' 
declared here}}
   int b1 = sizeof(arr4) / sizeof(arr2[12]); // expected-warning {{expression 
does not compute the number of elements in this array; element type is 'int 
[12]', not 'unsigned long long'}}
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -9191,6 +9191,7 @@
   } else if (const auto *ArrayTy = S.Context.getAsArrayType(LHSTy)) {
 QualType ArrayElemTy = ArrayTy->getElementType();
 if (ArrayElemTy->isDependentType() || RHSTy->isDependentType() ||
+ArrayElemTy->isCharType() ||
 S.Context.getTypeSize(ArrayElemTy) == S.Context.getTypeSize(RHSTy))
   return;
 S.Diag(Loc, diag::warn_division_sizeof_array)


Index: clang/test/Sema/div-sizeof-array.cpp
===
--- clang/test/Sema/div-sizeof-array.cpp
+++ clang/test/Sema/div-sizeof-array.cpp
@@ -21,6 +21,8 @@
   int a9 = sizeof(arr) / sizeof(unsigned int);
   const char arr3[2] = "A";
   int a10 = sizeof(arr3) / sizeof(char);
+  int a11 = sizeof(arr3) / sizeof(p);
+  int a12 = sizeof(arr3) / sizeof(int);
 
   int arr4[10][12]; // expected-note 3 {{array 'arr4' declared here}}
   int b1 = sizeof(arr4) / sizeof(arr2[12]); // expected-warning {{expression does not compute the number of elements in this array; element type is 'int [12]', not 'unsigned long long'}}
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -9191,6 +9191,7 @@
   } else if (const auto *ArrayTy = S.Context.getAsArrayType(LHSTy)) {
 QualType ArrayElemTy = ArrayTy->getElementType();
 if (ArrayElemTy->isDependentType() || RHSTy->isDependentType() ||
+ArrayElemTy->isCharType() ||
 S.Context.getTypeSize(ArrayElemTy) == S.Context.getTypeSize(RHSTy))
   return;
 S.Diag(Loc, diag::warn_division_sizeof_array)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D68368: [ItaniumMangle] Fix mangling of GNU __null in an expression to match GCC

[James Clarke via Phabricator via cfe-commits]

jrtc27 created this revision.
jrtc27 added a reviewer: rsmith.
Herald added subscribers: cfe-commits, erik.pilkington.
Herald added a project: clang.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D68368

Files:
  clang/lib/AST/ItaniumMangle.cpp
  clang/test/CodeGenCXX/mangle-exprs.cpp


Index: clang/test/CodeGenCXX/mangle-exprs.cpp
===
--- clang/test/CodeGenCXX/mangle-exprs.cpp
+++ clang/test/CodeGenCXX/mangle-exprs.cpp
@@ -373,3 +373,19 @@
   template void f(decltype(T{.a.b[3][1 ... 4] = 9}) x) {}
   void use_f(A a) { f(a); }
 }
+
+namespace null {
+  template 
+  void cpp_nullptr(typename enable_if::type* = 0) {
+  }
+
+  template 
+  void gnu_null(typename enable_if::type* = 0) {
+  }
+
+  // CHECK-LABEL: define {{.*}} 
@_ZN4null11cpp_nullptrILDn0EEEvPN9enable_ifIXeqT_LDnEEvE4typeE
+  template void cpp_nullptr(void *);
+
+  // CHECK-LABEL: define {{.*}} 
@_ZN4null8gnu_nullILPv0EEEvPN9enable_ifIXeqT_Ll0EEvE4typeE
+  template void gnu_null(void *);
+}
Index: clang/lib/AST/ItaniumMangle.cpp
===
--- clang/lib/AST/ItaniumMangle.cpp
+++ clang/lib/AST/ItaniumMangle.cpp
@@ -4273,8 +4273,11 @@
   }
 
   case Expr::GNUNullExprClass:
-// FIXME: should this really be mangled the same as nullptr?
-// fallthrough
+// Mangle as if an integer literal 0.
+Out << 'L';
+mangleType(E->getType());
+Out << "0E";
+break;
 
   case Expr::CXXNullPtrLiteralExprClass: {
 Out << "LDnE";


Index: clang/test/CodeGenCXX/mangle-exprs.cpp
===
--- clang/test/CodeGenCXX/mangle-exprs.cpp
+++ clang/test/CodeGenCXX/mangle-exprs.cpp
@@ -373,3 +373,19 @@
   template void f(decltype(T{.a.b[3][1 ... 4] = 9}) x) {}
   void use_f(A a) { f(a); }
 }
+
+namespace null {
+  template 
+  void cpp_nullptr(typename enable_if::type* = 0) {
+  }
+
+  template 
+  void gnu_null(typename enable_if::type* = 0) {
+  }
+
+  // CHECK-LABEL: define {{.*}} @_ZN4null11cpp_nullptrILDn0EEEvPN9enable_ifIXeqT_LDnEEvE4typeE
+  template void cpp_nullptr(void *);
+
+  // CHECK-LABEL: define {{.*}} @_ZN4null8gnu_nullILPv0EEEvPN9enable_ifIXeqT_Ll0EEvE4typeE
+  template void gnu_null(void *);
+}
Index: clang/lib/AST/ItaniumMangle.cpp
===
--- clang/lib/AST/ItaniumMangle.cpp
+++ clang/lib/AST/ItaniumMangle.cpp
@@ -4273,8 +4273,11 @@
   }
 
   case Expr::GNUNullExprClass:
-// FIXME: should this really be mangled the same as nullptr?
-// fallthrough
+// Mangle as if an integer literal 0.
+Out << 'L';
+mangleType(E->getType());
+Out << "0E";
+break;
 
   case Expr::CXXNullPtrLiteralExprClass: {
 Out << "LDnE";
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D66324: clang-misexpect: Profile Guided Validation of Performance Annotations in LLVM

[James Clarke via Phabricator via cfe-commits]

jrtc27 added inline comments.



Comment at: llvm/trunk/include/llvm/IR/DiagnosticInfo.h:79
+  DK_FirstPluginKind,
+  DK_MisExpect
 };

Is this not going to clash with the first caller to 
`getNextAvailablePluginDiagnosticKind`? `DK_FirstPluginKind` is special and 
shouldn't have anything after it.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D66324



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

[PATCH] D58531: [clang] Specify type of pthread_create builtin

[James Clarke via Phabricator via cfe-commits]

jrtc27 added a comment.

In D58531#1611356 , @jdoerfert wrote:

> In D58531#1599209 , @probinson wrote:
>
> > We've started running into this too in building the PS4 system. +jdoerfert 
> > who added pthread_create to the builtin list.
> >
> > Looking at the patch, it seems straightforward enough although clearly 
> > needs clang-format-diff run over it.
> >  I don't touch Clang that much so I'm reluctant to okay it myself.
>
>
> @probinson Thanks for making me aware of this patch.
>
> > A separate point is whether it makes sense to be emitting this warning in 
> > the first place for GE_Missing_type. I'd argue that, if we don't know the 
> > type of the builtin, we should never emit the warning
>
> @jrtc27 I hope the immediate need for this is gone after D58091 
>  was finally committed? Given that I caused 
> this mess I can take a look at the patch if you are still interested in it, 
> are you?


Yes, now that it's not a warning if we don't provide a type this is no longer 
completely necessary. I would still be slightly in favour of this patch though 
as then we can detect dodgy `pthread_create` declarations, but it's not a big 
deal.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D58531



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

[PATCH] D67185: [RISCV] Add support for -ffixed-xX flags

[James Clarke via Phabricator via cfe-commits]

jrtc27 added inline comments.



Comment at: llvm/test/CodeGen/RISCV/reserved-regs.ll:71
+
+; X1-NOT: lw ra,
+; X1-NOT: ld ra,

lenary wrote:
> These tests aren't going to test what you think they are, or at least aren't 
> going to fail when you hope they are, as the allocator will probably choose 
> a0 for these registers, even if other registers are available (it uses the 
> GPRs in a specific order, starting with the first free one, which will 
> usually be a0).
This is approximately what `llvm/test/CodeGen/AArch64/arm64-platform-reg.ll` is 
doing, and the pint is that `@var` is big enough that the register allocator 
will use every register it can so the order shouldn't matter.

As for reserving registers that have a specific use in the calling convention, 
AArch64 allows you to use them but won't let you make function calls 
(`llvm/test/CodeGen/AArch64/arm64-reserved-arg-reg-call-error.ll`). I don't 
know what it does about function arguments though, it probably assumes they're 
in the ABI registers on entry but then doesn't allocate/clobber those registers 
within the function? I'm not quite sure what that's useful for though. GCC has 
this to say about the flag:

> Treat the register named reg as a fixed register; generated code should never 
> refer to it (except perhaps as a stack pointer, frame pointer or in some 
> other fixed role).

So it's pretty vague about what can happen if you try and reserve a register 
normally used for other things...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67185



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

[PATCH] D57795: [RISCV] Add FreeBSD targets

[James Clarke via Phabricator via cfe-commits]

jrtc27 updated this revision to Diff 203359.
jrtc27 added a comment.

Sorted OS case statements


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D57795

Files:
  clang/lib/Basic/Targets.cpp
  clang/lib/Driver/ToolChains/FreeBSD.cpp
  clang/test/Driver/freebsd.c


Index: clang/test/Driver/freebsd.c
===
--- clang/test/Driver/freebsd.c
+++ clang/test/Driver/freebsd.c
@@ -63,6 +63,15 @@
 // RUN:   | FileCheck --check-prefix=CHECK-MIPSN32EL-LD %s
 // CHECK-MIPSN32EL-LD: ld{{.*}}" {{.*}} "-m" "elf32ltsmipn32_fbsd"
 //
+// Check that RISC-V passes the correct linker emulation.
+//
+// RUN: %clang -target riscv32-freebsd %s -### %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-RV32I-LD %s
+// CHECK-RV32I-LD: ld{{.*}}" {{.*}} "-m" "elf32lriscv"
+// RUN: %clang -target riscv64-freebsd %s -### %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-RV64I-LD %s
+// CHECK-RV64I-LD: ld{{.*}}" {{.*}} "-m" "elf64lriscv"
+//
 // Check that the new linker flags are passed to FreeBSD
 // RUN: %clang -no-canonical-prefixes -target x86_64-pc-freebsd8 -m32 %s \
 // RUN:   --sysroot=%S/Inputs/multiarch_freebsd64_tree -### 2>&1 \
Index: clang/lib/Driver/ToolChains/FreeBSD.cpp
===
--- clang/lib/Driver/ToolChains/FreeBSD.cpp
+++ clang/lib/Driver/ToolChains/FreeBSD.cpp
@@ -197,6 +197,14 @@
 else
   CmdArgs.push_back("elf64ltsmip_fbsd");
 break;
+  case llvm::Triple::riscv32:
+CmdArgs.push_back("-m");
+CmdArgs.push_back("elf32lriscv");
+break;
+  case llvm::Triple::riscv64:
+CmdArgs.push_back("-m");
+CmdArgs.push_back("elf64lriscv");
+break;
   default:
 break;
   }
Index: clang/lib/Basic/Targets.cpp
===
--- clang/lib/Basic/Targets.cpp
+++ clang/lib/Basic/Targets.cpp
@@ -376,15 +376,26 @@
 return new AMDGPUTargetInfo(Triple, Opts);
 
   case llvm::Triple::riscv32:
-// TODO: add cases for FreeBSD, NetBSD, RTEMS once tested.
-if (os == llvm::Triple::Linux)
+// TODO: add cases for NetBSD, RTEMS once tested.
+switch (os) {
+case llvm::Triple::FreeBSD:
+  return new FreeBSDTargetInfo(Triple, Opts);
+case llvm::Triple::Linux:
   return new LinuxTargetInfo(Triple, Opts);
-return new RISCV32TargetInfo(Triple, Opts);
+default:
+  return new RISCV32TargetInfo(Triple, Opts);
+}
+
   case llvm::Triple::riscv64:
-// TODO: add cases for FreeBSD, NetBSD, RTEMS once tested.
-if (os == llvm::Triple::Linux)
+// TODO: add cases for NetBSD, RTEMS once tested.
+switch (os) {
+case llvm::Triple::FreeBSD:
+  return new FreeBSDTargetInfo(Triple, Opts);
+case llvm::Triple::Linux:
   return new LinuxTargetInfo(Triple, Opts);
-return new RISCV64TargetInfo(Triple, Opts);
+default:
+  return new RISCV64TargetInfo(Triple, Opts);
+}
 
   case llvm::Triple::sparc:
 switch (os) {


Index: clang/test/Driver/freebsd.c
===
--- clang/test/Driver/freebsd.c
+++ clang/test/Driver/freebsd.c
@@ -63,6 +63,15 @@
 // RUN:   | FileCheck --check-prefix=CHECK-MIPSN32EL-LD %s
 // CHECK-MIPSN32EL-LD: ld{{.*}}" {{.*}} "-m" "elf32ltsmipn32_fbsd"
 //
+// Check that RISC-V passes the correct linker emulation.
+//
+// RUN: %clang -target riscv32-freebsd %s -### %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-RV32I-LD %s
+// CHECK-RV32I-LD: ld{{.*}}" {{.*}} "-m" "elf32lriscv"
+// RUN: %clang -target riscv64-freebsd %s -### %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-RV64I-LD %s
+// CHECK-RV64I-LD: ld{{.*}}" {{.*}} "-m" "elf64lriscv"
+//
 // Check that the new linker flags are passed to FreeBSD
 // RUN: %clang -no-canonical-prefixes -target x86_64-pc-freebsd8 -m32 %s \
 // RUN:   --sysroot=%S/Inputs/multiarch_freebsd64_tree -### 2>&1 \
Index: clang/lib/Driver/ToolChains/FreeBSD.cpp
===
--- clang/lib/Driver/ToolChains/FreeBSD.cpp
+++ clang/lib/Driver/ToolChains/FreeBSD.cpp
@@ -197,6 +197,14 @@
 else
   CmdArgs.push_back("elf64ltsmip_fbsd");
 break;
+  case llvm::Triple::riscv32:
+CmdArgs.push_back("-m");
+CmdArgs.push_back("elf32lriscv");
+break;
+  case llvm::Triple::riscv64:
+CmdArgs.push_back("-m");
+CmdArgs.push_back("elf64lriscv");
+break;
   default:
 break;
   }
Index: clang/lib/Basic/Targets.cpp
===
--- clang/lib/Basic/Targets.cpp
+++ clang/lib/Basic/Targets.cpp
@@ -376,15 +376,26 @@
 return new AMDGPUTargetInfo(Triple, Opts);
 
   case llvm::Triple::riscv32:
-// TODO: add cases for FreeBSD, NetBSD, RTEMS once tested.
-if (os == llvm::Triple::Linux)
+// TODO: add cases for NetBSD, RTEMS once tested.
+switc

[PATCH] D61464: [RiscV] Typo in register aliases

[James Clarke via Phabricator via cfe-commits]

jrtc27 added a comment.

Ouch; LGTM.


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

https://reviews.llvm.org/D61464



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

[PATCH] D58531: [clang] Specify type of pthread_create builtin

[James Clarke via Phabricator via cfe-commits]

jrtc27 added a comment.

A separate point is whether it makes sense to be emitting this warning in the 
first place for `GE_Missing_type`. I'd argue that, if we don't know the type of 
the builtin, we should never emit the warning, since otherwise we always emit 
the warning even for legitimate instances, though that's not really important 
now we're not hitting that case any more.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D58531



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

[PATCH] D58531: [clang] Specify type of pthread_create builtin

[James Clarke via Phabricator via cfe-commits]

jrtc27 created this revision.
Herald added subscribers: cfe-commits, jfb.
Herald added a project: clang.

Currently, pthread_create's type string is empty, so GetBuiltinType
fails and any header declaring it gives a -Wbuiltin-requires-header
warning, which gives a false-positive when including pthread.h itself
with -Wsystem-headers. By specifying its type, this false-positive goes
away and it behaves like every other builtin declared in system headers,
only warning when the declaration does not match the expected type.

This requires introducing a way to declare function pointer types in
builtin type strings, which requires refactoring GetBuiltinType to allow
recursive parsing of function types, expressed as a type string within
parentheses.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D58531

Files:
  clang/include/clang/AST/ASTContext.h
  clang/include/clang/Basic/Builtins.def
  clang/include/clang/Serialization/ASTBitCodes.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTWriter.cpp
  clang/test/Analysis/retain-release.m
  clang/test/Sema/implicit-builtin-decl.c

Index: clang/test/Sema/implicit-builtin-decl.c
===
--- clang/test/Sema/implicit-builtin-decl.c
+++ clang/test/Sema/implicit-builtin-decl.c
@@ -66,3 +66,5 @@
 // CHECK: FunctionDecl {{.*}}  col:6 sigsetjmp '
 // CHECK-NOT: FunctionDecl
 // CHECK: ReturnsTwiceAttr {{.*}} <{{.*}}> Implicit
+
+int pthread_create(); // expected-warning{{declaration of built-in function 'pthread_create' requires inclusion of the header }}
Index: clang/test/Analysis/retain-release.m
===
--- clang/test/Analysis/retain-release.m
+++ clang/test/Analysis/retain-release.m
@@ -1231,7 +1231,7 @@
 typedef unsigned long __darwin_pthread_key_t;
 typedef __darwin_pthread_key_t pthread_key_t;
 
-int pthread_create(pthread_t *, const pthread_attr_t *,  // C-warning{{declaration of built-in function 'pthread_create' requires inclusion of the header }}
+int pthread_create(pthread_t *, const pthread_attr_t *,  // no-warning
void *(*)(void *), void *);
 
 int pthread_setspecific(pthread_key_t key, const void *value);
Index: clang/lib/Serialization/ASTWriter.cpp
===
--- clang/lib/Serialization/ASTWriter.cpp
+++ clang/lib/Serialization/ASTWriter.cpp
@@ -4904,6 +4904,8 @@
   AddTypeRef(Context.ObjCClassRedefinitionType, SpecialTypes);
   AddTypeRef(Context.ObjCSelRedefinitionType, SpecialTypes);
   AddTypeRef(Context.getucontext_tType(), SpecialTypes);
+  AddTypeRef(Context.getpthread_tType(), SpecialTypes);
+  AddTypeRef(Context.getpthread_attr_tType(), SpecialTypes);
 
   if (Chain) {
 // Write the mapping information describing our module dependencies and how
Index: clang/lib/Serialization/ASTReader.cpp
===
--- clang/lib/Serialization/ASTReader.cpp
+++ clang/lib/Serialization/ASTReader.cpp
@@ -4560,6 +4560,42 @@
 }
   }
 }
+
+if (unsigned Pthread_t = SpecialTypes[SPECIAL_TYPE_PTHREAD_T]) {
+  QualType Pthread_tType = GetType(Pthread_t);
+  if (Pthread_tType.isNull()) {
+Error("pthread_t type is NULL");
+return;
+  }
+
+  if (!Context.pthread_tDecl) {
+if (const TypedefType *Typedef = Pthread_tType->getAs())
+  Context.setpthread_tDecl(Typedef->getDecl());
+else {
+  const TagType *Tag = Pthread_tType->getAs();
+  assert(Tag && "Invalid pthread_t type in AST file");
+  Context.setpthread_tDecl(Tag->getDecl());
+}
+  }
+}
+
+if (unsigned Pthread_attr_t = SpecialTypes[SPECIAL_TYPE_PTHREAD_ATTR_T]) {
+  QualType Pthread_attr_tType = GetType(Pthread_attr_t);
+  if (Pthread_attr_tType.isNull()) {
+Error("pthread_attr_t type is NULL");
+return;
+  }
+
+  if (!Context.pthread_attr_tDecl) {
+if (const TypedefType *Typedef = Pthread_attr_tType->getAs())
+  Context.setpthread_attr_tDecl(Typedef->getDecl());
+else {
+  const TagType *Tag = Pthread_attr_tType->getAs();
+  assert(Tag && "Invalid pthread_attr_t type in AST file");
+  Context.setpthread_attr_tDecl(Tag->getDecl());
+}
+  }
+}
   }
 
   ReadPragmaDiagnosticMappings(Context.getDiagnostics());
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -1943,6 +1943,8 @@
 return "setjmp.h";
   case ASTContext::GE_Missing_ucontext:
 return "ucontext.h";
+  case ASTContext::GE_Missing_pthread:
+return "pthread.h";
   }
   llvm_unreachable("unhandled error kind");
 }
@@ -5809,6 +5811,10 @@
 Context.setsigjmp_bufDecl(N

[PATCH] D57450: [RISCV] Set MaxAtomicInlineWidth and MaxAtomicPromoteWidth for RV32/RV64 targets with atomics

[James Clarke via Phabricator via cfe-commits]

jrtc27 added a comment.

Also worth noting that the frontend needs to know the max width otherwise 
`warn_atomic_op_misaligned` will be triggered with the message `large atomic 
operation may incur significant performance penalty` (name is misleading since 
the `%select{large|misaligned}0` means it's triggered for both large and 
misaligned atomic ops, and it's also in the `atomic-alignment` group). This is 
exactly what I did in my tree (other than the tests) and it seemed to work 
correctly for RV64A compiling FreeBSD.


Repository:
  rC Clang

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

https://reviews.llvm.org/D57450



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

[PATCH] D54295: [WIP, RISCV] Add inline asm constraint A for RISC-V

[James Clarke via Phabricator via cfe-commits]

jrtc27 requested changes to this revision.
jrtc27 added inline comments.
This revision now requires changes to proceed.
Herald added a project: clang.



Comment at: test/CodeGen/riscv-inline-asm.c:32
+// CHECK-LABEL: define void @test_A(i32* %p)
+// CHECK: call void asm volatile "", "*A"(i32* %p)
+  asm volatile("" :: "A"(*p));

Should be `sideeffect`


Repository:
  rC Clang

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

https://reviews.llvm.org/D54295



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

[PATCH] D57795: [RISCV] Add FreeBSD targets

[James Clarke via Phabricator via cfe-commits]

jrtc27 created this revision.
jrtc27 added a reviewer: asb.
Herald added subscribers: cfe-commits, rkruppe, rogfer01, shiva0217, 
kito-cheng, emaste.
Herald added a project: clang.

Repository:
  rC Clang

https://reviews.llvm.org/D57795

Files:
  lib/Basic/Targets.cpp
  lib/Driver/ToolChains/FreeBSD.cpp
  test/Driver/freebsd.c


Index: test/Driver/freebsd.c
===
--- test/Driver/freebsd.c
+++ test/Driver/freebsd.c
@@ -63,6 +63,15 @@
 // RUN:   | FileCheck --check-prefix=CHECK-MIPSN32EL-LD %s
 // CHECK-MIPSN32EL-LD: ld{{.*}}" {{.*}} "-m" "elf32ltsmipn32_fbsd"
 //
+// Check that RISC-V passes the correct linker emulation.
+//
+// RUN: %clang -target riscv32-freebsd %s -### %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-RV32I-LD %s
+// CHECK-RV32I-LD: ld{{.*}}" {{.*}} "-m" "elf32lriscv"
+// RUN: %clang -target riscv64-freebsd %s -### %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-RV64I-LD %s
+// CHECK-RV64I-LD: ld{{.*}}" {{.*}} "-m" "elf64lriscv"
+//
 // Check that the new linker flags are passed to FreeBSD
 // RUN: %clang -no-canonical-prefixes -target x86_64-pc-freebsd8 -m32 %s \
 // RUN:   --sysroot=%S/Inputs/multiarch_freebsd64_tree -### 2>&1 \
Index: lib/Driver/ToolChains/FreeBSD.cpp
===
--- lib/Driver/ToolChains/FreeBSD.cpp
+++ lib/Driver/ToolChains/FreeBSD.cpp
@@ -197,6 +197,14 @@
 else
   CmdArgs.push_back("elf64ltsmip_fbsd");
 break;
+  case llvm::Triple::riscv32:
+CmdArgs.push_back("-m");
+CmdArgs.push_back("elf32lriscv");
+break;
+  case llvm::Triple::riscv64:
+CmdArgs.push_back("-m");
+CmdArgs.push_back("elf64lriscv");
+break;
   default:
 break;
   }
Index: lib/Basic/Targets.cpp
===
--- lib/Basic/Targets.cpp
+++ lib/Basic/Targets.cpp
@@ -372,15 +372,26 @@
 return new AMDGPUTargetInfo(Triple, Opts);
 
   case llvm::Triple::riscv32:
-// TODO: add cases for FreeBSD, NetBSD, RTEMS once tested.
-if (os == llvm::Triple::Linux)
+// TODO: add cases for NetBSD, RTEMS once tested.
+switch (os) {
+case llvm::Triple::Linux:
   return new LinuxTargetInfo(Triple, Opts);
-return new RISCV32TargetInfo(Triple, Opts);
+case llvm::Triple::FreeBSD:
+  return new FreeBSDTargetInfo(Triple, Opts);
+default:
+  return new RISCV32TargetInfo(Triple, Opts);
+}
+
   case llvm::Triple::riscv64:
-// TODO: add cases for FreeBSD, NetBSD, RTEMS once tested.
-if (os == llvm::Triple::Linux)
+// TODO: add cases for NetBSD, RTEMS once tested.
+switch (os) {
+case llvm::Triple::Linux:
   return new LinuxTargetInfo(Triple, Opts);
-return new RISCV64TargetInfo(Triple, Opts);
+case llvm::Triple::FreeBSD:
+  return new FreeBSDTargetInfo(Triple, Opts);
+default:
+  return new RISCV64TargetInfo(Triple, Opts);
+}
 
   case llvm::Triple::sparc:
 switch (os) {


Index: test/Driver/freebsd.c
===
--- test/Driver/freebsd.c
+++ test/Driver/freebsd.c
@@ -63,6 +63,15 @@
 // RUN:   | FileCheck --check-prefix=CHECK-MIPSN32EL-LD %s
 // CHECK-MIPSN32EL-LD: ld{{.*}}" {{.*}} "-m" "elf32ltsmipn32_fbsd"
 //
+// Check that RISC-V passes the correct linker emulation.
+//
+// RUN: %clang -target riscv32-freebsd %s -### %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-RV32I-LD %s
+// CHECK-RV32I-LD: ld{{.*}}" {{.*}} "-m" "elf32lriscv"
+// RUN: %clang -target riscv64-freebsd %s -### %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-RV64I-LD %s
+// CHECK-RV64I-LD: ld{{.*}}" {{.*}} "-m" "elf64lriscv"
+//
 // Check that the new linker flags are passed to FreeBSD
 // RUN: %clang -no-canonical-prefixes -target x86_64-pc-freebsd8 -m32 %s \
 // RUN:   --sysroot=%S/Inputs/multiarch_freebsd64_tree -### 2>&1 \
Index: lib/Driver/ToolChains/FreeBSD.cpp
===
--- lib/Driver/ToolChains/FreeBSD.cpp
+++ lib/Driver/ToolChains/FreeBSD.cpp
@@ -197,6 +197,14 @@
 else
   CmdArgs.push_back("elf64ltsmip_fbsd");
 break;
+  case llvm::Triple::riscv32:
+CmdArgs.push_back("-m");
+CmdArgs.push_back("elf32lriscv");
+break;
+  case llvm::Triple::riscv64:
+CmdArgs.push_back("-m");
+CmdArgs.push_back("elf64lriscv");
+break;
   default:
 break;
   }
Index: lib/Basic/Targets.cpp
===
--- lib/Basic/Targets.cpp
+++ lib/Basic/Targets.cpp
@@ -372,15 +372,26 @@
 return new AMDGPUTargetInfo(Triple, Opts);
 
   case llvm::Triple::riscv32:
-// TODO: add cases for FreeBSD, NetBSD, RTEMS once tested.
-if (os == llvm::Triple::Linux)
+// TODO: add cases for NetBSD, RTEMS once tested.
+switch (os) {
+case llvm::Triple::Linux:
   return new LinuxTargetInfo(Triple, Opts);
-return new RISCV32TargetInfo(

[PATCH] D57497: [RISCV] Passing small data limitation value to RISCV backend

[James Clarke via Phabricator via cfe-commits]

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

Please update `docs/ClangCommandLineReference.rst`. Also, in my limited 
testing, GCC just seems to pass the `-msmall-data-limit=...` flag straight 
through to the linker through `COLLECT_GCC_OPTIONS`. Finally, please add tests 
for `-msmall-data-limit=...` instead of or as well as `-G`, since GCC only 
recognises the former on RISC-V.


Repository:
  rC Clang

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

https://reviews.llvm.org/D57497



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

[PATCH] D57055: [RISCV] Mark TLS as supported

[James Clarke via Phabricator via cfe-commits]

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



Comment at: lib/Basic/Targets/RISCV.h:39
 HasD(false), HasC(false) {
-TLSSupported = false;
+TLSSupported = true;
 LongDoubleWidth = 128;

With the exception of `SystemZTargetInfo`, all CPU target info classes rely on 
the base already initialising it to `true` by default, so you can just delete 
this line.


Repository:
  rC Clang

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

https://reviews.llvm.org/D57055



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

[PATCH] D55326: [Driver] Fix incorrect GNU triplet for PowerPC on SUSE Linux

[James Clarke via Phabricator via cfe-commits]

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

Yes, this is a stupid situation to be in, but 32-bit PowerPC on SUSE really 
does use `/usr/lib/gcc/powerpc64-suse-linux`:

  
root@redpanda:/var/tmp/build-root/openSUSE_Factory_PowerPC-ppc/usr/lib/gcc/powerpc64-suse-linux/8#
 file crtbegin.o
  crtbegin.o: ELF 32-bit MSB relocatable, PowerPC or cisco 4500, version 1 
(SYSV), not stripped

I would however argue that we should keep `powerpc-suse-linux`; in theory 
there's nothing stopping a SUSE derivative (or SUSE themselves if they want to 
do a mass-rebuild) from moving everything to `powerpc-suse-linux` and improving 
everyone's sanity.

This should also have a comment explaining why there's a 64-bit triple here for 
future readers.


Repository:
  rC Clang

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

https://reviews.llvm.org/D55326



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

[PATCH] D53392: [RISCV] Collect library directories and triples for riscv64 triple too

[James Clarke via Phabricator via cfe-commits]

jrtc27 added inline comments.



Comment at: test/Driver/riscv64-toolchain.c:71
+// RUN: %clang %s -### -no-canonical-prefixes -fuse-ld=ld \
+// RUN:   -target riscv64-linux-unknown-elf \
+// RUN:   --gcc-toolchain=%S/Inputs/multilib_riscv_linux_sdk \

This (and below) are the only instances of `CPU-linux-unknown-elf` in the whole 
of clang (and there are only two instances of `CPU-linux-elf`, both in 
`test/Modules/pch_container.m`), apart from the riscv32 ones they were copied 
from. Perhaps they should be `riscv64-linux-unknown-gnu` instead? Notably, when 
parsed as `CPU-company-kernel-OS` this in fact sets *company* to linux and 
*kernel* to unknown, whereas if you really mean the full 4-tuple should the 
linux and unknown not be interchanged?



Comment at: test/Driver/riscv64-toolchain.c:87
+// RUN: %clang %s -### -no-canonical-prefixes -fuse-ld=ld \
+// RUN:   -target riscv64-linux-unknown-elf -march=rv64imafd -mabi=lp64d \
+// RUN:   --gcc-toolchain=%S/Inputs/multilib_riscv_linux_sdk \

`riscv64-linux-unknown-gnu` as before.


Repository:
  rC Clang

https://reviews.llvm.org/D53392



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

[PATCH] D53392: [RISCV] Collect library directories and triples for riscv64 triple too

[James Clarke via Phabricator via cfe-commits]

jrtc27 added inline comments.



Comment at: lib/Driver/ToolChains/Gnu.cpp:1912
 
-  static const char *const RISCV32LibDirs[] = {"/lib", "/lib32"};
+  static const char *const RISCVLibDirs[] = {"/lib", "/lib32"};
   static const char *const RISCVTriples[] = {"riscv32-unknown-linux-gnu",

Surely this should remain as `RISCV32LibDirs`? Also, should we reverse the 
order (put `"/lib32"` first) to match every single other architecture?



Comment at: lib/Driver/ToolChains/Gnu.cpp:1913
+  static const char *const RISCVLibDirs[] = {"/lib", "/lib32"};
   static const char *const RISCVTriples[] = {"riscv32-unknown-linux-gnu",
  "riscv32-unknown-elf"};

`RISCV32Triples`? Also, why do we have no `"riscv32-linux-gnu"` entry like the 
other architectures?



Comment at: lib/Driver/ToolChains/Gnu.cpp:1915
  "riscv32-unknown-elf"};
+  static const char *const RISCV64LibDirs[] = {"/lib", "/lib64"};
+  static const char *const RISCV64Triples[] = {"riscv64-unknown-linux-gnu",

Order as for 32.



Comment at: lib/Driver/ToolChains/Gnu.cpp:1916
+  static const char *const RISCV64LibDirs[] = {"/lib", "/lib64"};
+  static const char *const RISCV64Triples[] = {"riscv64-unknown-linux-gnu",
+   "riscv64-unknown-elf"};

`riscv64-linux-gnu`?


Repository:
  rC Clang

https://reviews.llvm.org/D53392



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

[PATCH] D43630: [Driver] Fix search paths on x32

[James Clarke via Phabricator via cfe-commits]

jrtc27 added a comment.

Ping?


Repository:
  rC Clang

https://reviews.llvm.org/D43630



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

[PATCH] D47137: [Sparc] Add floating-point register names

[James Clarke via Phabricator via cfe-commits]

jrtc27 added inline comments.



Comment at: lib/Basic/Targets/Sparc.cpp:52
+
+// Double precision floating-point register
+{{"d0"}, "f0"},   {{"d1"}, "f2"},   {{"d2"}, "f4"},   {{"d3"}, "f6"},

jyknight wrote:
> AFAICT, gcc doesn't actually accept "d" and "q" aliases in its inline asm, so 
> probably no point in LLVM doing so either.
Ah you're right; trying to use it gives "invalid register name". Having said 
that, maybe it's a nice thing to support anyway?


https://reviews.llvm.org/D47137



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

[PATCH] D47137: [Sparc] Add floating-point register names

[James Clarke via Phabricator via cfe-commits]

jrtc27 added a comment.

What about the V9 `dX` and `qX` aliases and `f32` to `f63`?


Repository:
  rC Clang

https://reviews.llvm.org/D47137



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

[PATCH] D43630: [Driver] Fix search paths on x32

[James Clarke via Phabricator via cfe-commits]

jrtc27 created this revision.
Herald added a subscriber: cfe-commits.

When targeting x32, the x32 libraries and headers should be used, not
the x86_64 ones (which may not even be available), so prioritise those
and use the right multiarch triple.


Repository:
  rC Clang

https://reviews.llvm.org/D43630

Files:
  lib/Driver/ToolChains/Gnu.cpp
  lib/Driver/ToolChains/Linux.cpp


Index: lib/Driver/ToolChains/Linux.cpp
===
--- lib/Driver/ToolChains/Linux.cpp
+++ lib/Driver/ToolChains/Linux.cpp
@@ -78,10 +78,13 @@
   return "i386-linux-gnu";
 break;
   case llvm::Triple::x86_64:
-// We don't want this for x32, otherwise it will match x86_64 libs
-if (TargetEnvironment != llvm::Triple::GNUX32 &&
-D.getVFS().exists(SysRoot + "/lib/x86_64-linux-gnu"))
-  return "x86_64-linux-gnu";
+if (TargetEnvironment == llvm::Triple::GNUX32) {
+  if (D.getVFS().exists(SysRoot + "/lib/x86_64-linux-gnux32"))
+return "x86_64-linux-gnux32";
+} else {
+  if (D.getVFS().exists(SysRoot + "/lib/x86_64-linux-gnu"))
+return "x86_64-linux-gnu";
+}
 break;
   case llvm::Triple::aarch64:
 if (D.getVFS().exists(SysRoot + "/lib/aarch64-linux-gnu"))
@@ -620,6 +623,8 @@
   // in use in any released version of Debian, so we should consider
   // removing them.
   "/usr/include/i686-linux-gnu/64", "/usr/include/i486-linux-gnu/64"};
+  const StringRef X32MultiarchIncludeDirs[] = {
+  "/usr/include/x86_64-linux-gnux32"};
   const StringRef X86MultiarchIncludeDirs[] = {
   "/usr/include/i386-linux-gnu",
 
@@ -661,7 +666,10 @@
   ArrayRef MultiarchIncludeDirs;
   switch (getTriple().getArch()) {
   case llvm::Triple::x86_64:
-MultiarchIncludeDirs = X86_64MultiarchIncludeDirs;
+if (getTriple().getEnvironment() == llvm::Triple::GNUX32)
+  MultiarchIncludeDirs = X32MultiarchIncludeDirs;
+else
+  MultiarchIncludeDirs = X86_64MultiarchIncludeDirs;
 break;
   case llvm::Triple::x86:
 MultiarchIncludeDirs = X86MultiarchIncludeDirs;
Index: lib/Driver/ToolChains/Gnu.cpp
===
--- lib/Driver/ToolChains/Gnu.cpp
+++ lib/Driver/ToolChains/Gnu.cpp
@@ -1848,7 +1848,10 @@
   "x86_64-manbo-linux-gnu", "x86_64-linux-gnu",
   "x86_64-slackware-linux", "x86_64-linux-android",
   "x86_64-unknown-linux"};
-  static const char *const X32LibDirs[] = {"/libx32"};
+  static const char *const X32LibDirs[] = {"/libx32", "/lib"};
+  static const char *const X32Triples[] = {
+  "x86_64-linux-gnux32","x86_64-unknown-linux-gnux32",
+  "x86_64-pc-linux-gnux32"};
   static const char *const X86LibDirs[] = {"/lib32", "/lib"};
   static const char *const X86Triples[] = {
   "i686-linux-gnu",   "i686-pc-linux-gnu", "i486-linux-gnu",
@@ -1989,14 +1992,16 @@
 }
 break;
   case llvm::Triple::x86_64:
-LibDirs.append(begin(X86_64LibDirs), end(X86_64LibDirs));
-TripleAliases.append(begin(X86_64Triples), end(X86_64Triples));
 // x32 is always available when x86_64 is available, so adding it as
 // secondary arch with x86_64 triples
 if (TargetTriple.getEnvironment() == llvm::Triple::GNUX32) {
-  BiarchLibDirs.append(begin(X32LibDirs), end(X32LibDirs));
+  LibDirs.append(begin(X32LibDirs), end(X32LibDirs));
+  TripleAliases.append(begin(X32Triples), end(X32Triples));
+  BiarchLibDirs.append(begin(X86_64LibDirs), end(X86_64LibDirs));
   BiarchTripleAliases.append(begin(X86_64Triples), end(X86_64Triples));
 } else {
+  LibDirs.append(begin(X86_64LibDirs), end(X86_64LibDirs));
+  TripleAliases.append(begin(X86_64Triples), end(X86_64Triples));
   BiarchLibDirs.append(begin(X86LibDirs), end(X86LibDirs));
   BiarchTripleAliases.append(begin(X86Triples), end(X86Triples));
 }


Index: lib/Driver/ToolChains/Linux.cpp
===
--- lib/Driver/ToolChains/Linux.cpp
+++ lib/Driver/ToolChains/Linux.cpp
@@ -78,10 +78,13 @@
   return "i386-linux-gnu";
 break;
   case llvm::Triple::x86_64:
-// We don't want this for x32, otherwise it will match x86_64 libs
-if (TargetEnvironment != llvm::Triple::GNUX32 &&
-D.getVFS().exists(SysRoot + "/lib/x86_64-linux-gnu"))
-  return "x86_64-linux-gnu";
+if (TargetEnvironment == llvm::Triple::GNUX32) {
+  if (D.getVFS().exists(SysRoot + "/lib/x86_64-linux-gnux32"))
+return "x86_64-linux-gnux32";
+} else {
+  if (D.getVFS().exists(SysRoot + "/lib/x86_64-linux-gnu"))
+return "x86_64-linux-gnu";
+}
 break;
   case llvm::Triple::aarch64:
 if (D.getVFS().exists(SysRoot + "/lib/aarch64-linux-gnu"))
@@ -620,6 +623,8 @@
   // in use in any released version of Debian, so we should consider
   // removing them.
   "/usr/include/i686-linux-gnu/64", "/usr/include/i486-linux-gnu/64"};
+  const String