[PATCH] D63498: [RISC-V] Add -msave-restore and -mno-save-restore to clang driver

[Sam Elliott via Phabricator via cfe-commits]

lenary created this revision.
lenary added reviewers: asb, luismarques.
Herald added subscribers: cfe-commits, jocewei, PkmX, rkruppe, the_o, 
brucehoult, MartinMosbeck, rogfer01, edward-jones, zzheng, jrtc27, shiva0217, 
kito-cheng, niosHD, sabuasal, apazos, simoncook, johnrusso, rbar.
Herald added a project: clang.

The GCC RISC-V toolchain accepts `-msave-restore` and `-mno-save-restore`
to control whether libcalls are used for saving and restoring the stack within
prologues and epilogues.

Clang currently errors if someone passes -msave-restore or -mno-save-restore.
This means that people need to change build configurations to use clang. This
patch adds these flags, so that clang invocations can now match gcc.

As the RISC-V backend does not currently have a `save-restore` target feature,
we emit a warning if someone requests `-msave-restore`. LLVM does not error if
we pass the (unimplemented) target features `+save-restore` or `-save-restore`.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D63498

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Arch/RISCV.cpp
  clang/test/Driver/riscv-features.c


Index: clang/test/Driver/riscv-features.c
===
--- clang/test/Driver/riscv-features.c
+++ clang/test/Driver/riscv-features.c
@@ -3,11 +3,20 @@
 
 // CHECK: fno-signed-char
 
+// RUN: %clang -target riscv32-unknown-elf -### %s 2>&1 | FileCheck %s 
-check-prefix=DEFAULT
+
 // RUN: %clang -target riscv32-unknown-elf -### %s -mrelax 2>&1 | FileCheck %s 
-check-prefix=RELAX
 // RUN: %clang -target riscv32-unknown-elf -### %s -mno-relax 2>&1 | FileCheck 
%s -check-prefix=NO-RELAX
-// RUN: %clang -target riscv32-unknown-elf -### %s 2>&1 | FileCheck %s 
-check-prefix=DEFAULT
 
 // RELAX: "-target-feature" "+relax"
 // NO-RELAX: "-target-feature" "-relax"
 // DEFAULT: "-target-feature" "+relax"
 // DEFAULT-NOT: "-target-feature" "-relax"
+
+// RUN: %clang -target riscv32-unknown-elf -### %s -msave-restore 2>&1 | 
FileCheck %s -check-prefix=SAVE-RESTORE
+// RUN: %clang -target riscv32-unknown-elf -### %s -mno-save-restore 2>&1 | 
FileCheck %s -check-prefix=NO-SAVE-RESTORE
+
+// SAVE-RESTORE: "-target-feature" "+save-restore"
+// NO-SAVE-RESTORE: "-target-feature" "-save-restore"
+// DEFAULT: "-target-feature" "-save-restore"
+// DEFAULT-NOT: "-target-feature" "+save-restore"
Index: clang/lib/Driver/ToolChains/Arch/RISCV.cpp
===
--- clang/lib/Driver/ToolChains/Arch/RISCV.cpp
+++ clang/lib/Driver/ToolChains/Arch/RISCV.cpp
@@ -358,6 +358,16 @@
   else
 Features.push_back("-relax");
 
+  // -mno-save-restore is default, unless -msave-restore is specified.
+  if (Args.hasFlag(options::OPT_msave_restore, options::OPT_mno_save_restore, 
false)) {
+Features.push_back("+save-restore");
+// ... but we don't yet support +save-restore, so issue a warning.
+D.Diag(diag::warn_drv_clang_unsupported)
+  << Args.getLastArg(options::OPT_msave_restore)->getAsString(Args);
+  } else {
+Features.push_back("-save-restore");
+  }
+
   // Now add any that the user explicitly requested on the command line,
   // which may override the defaults.
   handleTargetFeaturesGroup(Args, Features, 
options::OPT_m_riscv_Features_Group);
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -2125,6 +2125,10 @@
   HelpText<"Enable linker relaxation">;
 def mno_relax : Flag<["-"], "mno-relax">, Group,
   HelpText<"Disable linker relaxation">;
+def msave_restore : Flag<["-"], "msave-restore">, 
Group,
+  HelpText<"Enable using library calls for save and restore">;
+def mno_save_restore : Flag<["-"], "mno-save-restore">, 
Group,
+  HelpText<"Disable using library calls for save and restore">;
 
 def munaligned_access : Flag<["-"], "munaligned-access">, 
Group,
   HelpText<"Allow memory accesses to be unaligned (AArch32/AArch64 only)">;


Index: clang/test/Driver/riscv-features.c
===
--- clang/test/Driver/riscv-features.c
+++ clang/test/Driver/riscv-features.c
@@ -3,11 +3,20 @@
 
 // CHECK: fno-signed-char
 
+// RUN: %clang -target riscv32-unknown-elf -### %s 2>&1 | FileCheck %s -check-prefix=DEFAULT
+
 // RUN: %clang -target riscv32-unknown-elf -### %s -mrelax 2>&1 | FileCheck %s -check-prefix=RELAX
 // RUN: %clang -target riscv32-unknown-elf -### %s -mno-relax 2>&1 | FileCheck %s -check-prefix=NO-RELAX
-// RUN: %clang -target riscv32-unknown-elf -### %s 2>&1 | FileCheck %s -check-prefix=DEFAULT
 
 // RELAX: "-target-feature" "+relax"
 // NO-RELAX: "-target-feature" "-relax"
 // DEFAULT: "-target-feature" "+relax"
 // DEFAULT-NOT: "-target-feature" "-relax"
+
+// RUN: %clang -target riscv32-unknown-elf -### %s -msave-restore 2>&1 | FileCheck %s -check-pref

[PATCH] D63498: [RISC-V] Add -msave-restore and -mno-save-restore to clang driver

[Sam Elliott via Phabricator via cfe-commits]

lenary updated this revision to Diff 205846.
lenary added a comment.

- Add CHECK lines for warnings Hopefully these will work regardless of race 
conditions in the merging of stdout and stderr.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63498

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Arch/RISCV.cpp
  clang/test/Driver/riscv-features.c


Index: clang/test/Driver/riscv-features.c
===
--- clang/test/Driver/riscv-features.c
+++ clang/test/Driver/riscv-features.c
@@ -3,11 +3,23 @@
 
 // CHECK: fno-signed-char
 
+// RUN: %clang -target riscv32-unknown-elf -### %s 2>&1 | FileCheck %s 
-check-prefix=DEFAULT
+
 // RUN: %clang -target riscv32-unknown-elf -### %s -mrelax 2>&1 | FileCheck %s 
-check-prefix=RELAX
 // RUN: %clang -target riscv32-unknown-elf -### %s -mno-relax 2>&1 | FileCheck 
%s -check-prefix=NO-RELAX
-// RUN: %clang -target riscv32-unknown-elf -### %s 2>&1 | FileCheck %s 
-check-prefix=DEFAULT
 
 // RELAX: "-target-feature" "+relax"
 // NO-RELAX: "-target-feature" "-relax"
 // DEFAULT: "-target-feature" "+relax"
 // DEFAULT-NOT: "-target-feature" "-relax"
+
+// RUN: %clang -target riscv32-unknown-elf -### %s -msave-restore 2>&1 | 
FileCheck %s -check-prefix=SAVE-RESTORE
+// RUN: %clang -target riscv32-unknown-elf -### %s -mno-save-restore 2>&1 | 
FileCheck %s -check-prefix=NO-SAVE-RESTORE
+
+// SAVE-RESTORE: warning: the clang compiler does not support '-msave-restore'
+// DEFAULT-NOT: warning: the clang compiler does not support
+
+// SAVE-RESTORE: "-target-feature" "+save-restore"
+// NO-SAVE-RESTORE: "-target-feature" "-save-restore"
+// DEFAULT: "-target-feature" "-save-restore"
+// DEFAULT-NOT: "-target-feature" "+save-restore"
\ No newline at end of file
Index: clang/lib/Driver/ToolChains/Arch/RISCV.cpp
===
--- clang/lib/Driver/ToolChains/Arch/RISCV.cpp
+++ clang/lib/Driver/ToolChains/Arch/RISCV.cpp
@@ -358,6 +358,16 @@
   else
 Features.push_back("-relax");
 
+  // -mno-save-restore is default, unless -msave-restore is specified.
+  if (Args.hasFlag(options::OPT_msave_restore, options::OPT_mno_save_restore, 
false)) {
+Features.push_back("+save-restore");
+// ... but we don't yet support +save-restore, so issue a warning.
+D.Diag(diag::warn_drv_clang_unsupported)
+  << Args.getLastArg(options::OPT_msave_restore)->getAsString(Args);
+  } else {
+Features.push_back("-save-restore");
+  }
+
   // Now add any that the user explicitly requested on the command line,
   // which may override the defaults.
   handleTargetFeaturesGroup(Args, Features, 
options::OPT_m_riscv_Features_Group);
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -2125,6 +2125,10 @@
   HelpText<"Enable linker relaxation">;
 def mno_relax : Flag<["-"], "mno-relax">, Group,
   HelpText<"Disable linker relaxation">;
+def msave_restore : Flag<["-"], "msave-restore">, 
Group,
+  HelpText<"Enable using library calls for save and restore">;
+def mno_save_restore : Flag<["-"], "mno-save-restore">, 
Group,
+  HelpText<"Disable using library calls for save and restore">;
 
 def munaligned_access : Flag<["-"], "munaligned-access">, 
Group,
   HelpText<"Allow memory accesses to be unaligned (AArch32/AArch64 only)">;


Index: clang/test/Driver/riscv-features.c
===
--- clang/test/Driver/riscv-features.c
+++ clang/test/Driver/riscv-features.c
@@ -3,11 +3,23 @@
 
 // CHECK: fno-signed-char
 
+// RUN: %clang -target riscv32-unknown-elf -### %s 2>&1 | FileCheck %s -check-prefix=DEFAULT
+
 // RUN: %clang -target riscv32-unknown-elf -### %s -mrelax 2>&1 | FileCheck %s -check-prefix=RELAX
 // RUN: %clang -target riscv32-unknown-elf -### %s -mno-relax 2>&1 | FileCheck %s -check-prefix=NO-RELAX
-// RUN: %clang -target riscv32-unknown-elf -### %s 2>&1 | FileCheck %s -check-prefix=DEFAULT
 
 // RELAX: "-target-feature" "+relax"
 // NO-RELAX: "-target-feature" "-relax"
 // DEFAULT: "-target-feature" "+relax"
 // DEFAULT-NOT: "-target-feature" "-relax"
+
+// RUN: %clang -target riscv32-unknown-elf -### %s -msave-restore 2>&1 | FileCheck %s -check-prefix=SAVE-RESTORE
+// RUN: %clang -target riscv32-unknown-elf -### %s -mno-save-restore 2>&1 | FileCheck %s -check-prefix=NO-SAVE-RESTORE
+
+// SAVE-RESTORE: warning: the clang compiler does not support '-msave-restore'
+// DEFAULT-NOT: warning: the clang compiler does not support
+
+// SAVE-RESTORE: "-target-feature" "+save-restore"
+// NO-SAVE-RESTORE: "-target-feature" "-save-restore"
+// DEFAULT: "-target-feature" "-save-restore"
+// DEFAULT-NOT: "-target-feature" "+save-restore"
\ No newline at end of file
Index: clang/li

[PATCH] D64737: RISCV: Add support for floating point registers in inlineasm

[Sam Elliott via Phabricator via cfe-commits]

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

Looks good to me! Thank you!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64737



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

[PATCH] D65500: [RISCV] Support 'f' Inline Assembly Constraint

[Sam Elliott via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL367403: [RISCV] Support 'f' Inline Assembly 
Constraint (authored by lenary, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D65500?vs=212513&id=212543#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D65500

Files:
  cfe/trunk/lib/Basic/Targets/RISCV.cpp
  cfe/trunk/test/CodeGen/riscv-inline-asm.c
  llvm/trunk/lib/Target/RISCV/RISCVISelLowering.cpp
  llvm/trunk/lib/Target/RISCV/RISCVISelLowering.h
  llvm/trunk/test/CodeGen/RISCV/inline-asm-d-constraint-f.ll
  llvm/trunk/test/CodeGen/RISCV/inline-asm-f-constraint-f.ll
  llvm/trunk/test/CodeGen/RISCV/inline-asm-invalid.ll

Index: llvm/trunk/test/CodeGen/RISCV/inline-asm-f-constraint-f.ll
===
--- llvm/trunk/test/CodeGen/RISCV/inline-asm-f-constraint-f.ll
+++ llvm/trunk/test/CodeGen/RISCV/inline-asm-f-constraint-f.ll
@@ -0,0 +1,34 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc -mtriple=riscv32 -mattr=+f -verify-machineinstrs < %s \
+; RUN:   | FileCheck -check-prefix=RV32F %s
+; RUN: llc -mtriple=riscv64 -mattr=+f -verify-machineinstrs < %s \
+; RUN:   | FileCheck -check-prefix=RV64F %s
+
+@gf = external global float
+
+define float @constraint_f_float(float %a) nounwind {
+; RV32F-LABEL: constraint_f_float:
+; RV32F:   # %bb.0:
+; RV32F-NEXT:fmv.w.x ft0, a0
+; RV32F-NEXT:lui a0, %hi(gf)
+; RV32F-NEXT:flw ft1, %lo(gf)(a0)
+; RV32F-NEXT:#APP
+; RV32F-NEXT:fadd.s ft0, ft0, ft1
+; RV32F-NEXT:#NO_APP
+; RV32F-NEXT:fmv.x.w a0, ft0
+; RV32F-NEXT:ret
+;
+; RV64F-LABEL: constraint_f_float:
+; RV64F:   # %bb.0:
+; RV64F-NEXT:fmv.w.x ft0, a0
+; RV64F-NEXT:lui a0, %hi(gf)
+; RV64F-NEXT:flw ft1, %lo(gf)(a0)
+; RV64F-NEXT:#APP
+; RV64F-NEXT:fadd.s ft0, ft0, ft1
+; RV64F-NEXT:#NO_APP
+; RV64F-NEXT:fmv.x.w a0, ft0
+; RV64F-NEXT:ret
+  %1 = load float, float* @gf
+  %2 = tail call float asm "fadd.s $0, $1, $2", "=f,f,f"(float %a, float %1)
+  ret float %2
+}
Index: llvm/trunk/test/CodeGen/RISCV/inline-asm-d-constraint-f.ll
===
--- llvm/trunk/test/CodeGen/RISCV/inline-asm-d-constraint-f.ll
+++ llvm/trunk/test/CodeGen/RISCV/inline-asm-d-constraint-f.ll
@@ -0,0 +1,40 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc -mtriple=riscv32 -mattr=+d -verify-machineinstrs < %s \
+; RUN:   | FileCheck -check-prefix=RV32F %s
+; RUN: llc -mtriple=riscv64 -mattr=+d -verify-machineinstrs < %s \
+; RUN:   | FileCheck -check-prefix=RV64F %s
+
+@gd = external global double
+
+define double @constraint_f_double(double %a) nounwind {
+; RV32F-LABEL: constraint_f_double:
+; RV32F:   # %bb.0:
+; RV32F-NEXT:addi sp, sp, -16
+; RV32F-NEXT:sw a0, 8(sp)
+; RV32F-NEXT:sw a1, 12(sp)
+; RV32F-NEXT:fld ft0, 8(sp)
+; RV32F-NEXT:lui a0, %hi(gd)
+; RV32F-NEXT:fld ft1, %lo(gd)(a0)
+; RV32F-NEXT:#APP
+; RV32F-NEXT:fadd.d ft0, ft0, ft1
+; RV32F-NEXT:#NO_APP
+; RV32F-NEXT:fsd ft0, 8(sp)
+; RV32F-NEXT:lw a0, 8(sp)
+; RV32F-NEXT:lw a1, 12(sp)
+; RV32F-NEXT:addi sp, sp, 16
+; RV32F-NEXT:ret
+;
+; RV64F-LABEL: constraint_f_double:
+; RV64F:   # %bb.0:
+; RV64F-NEXT:fmv.d.x ft0, a0
+; RV64F-NEXT:lui a0, %hi(gd)
+; RV64F-NEXT:fld ft1, %lo(gd)(a0)
+; RV64F-NEXT:#APP
+; RV64F-NEXT:fadd.d ft0, ft0, ft1
+; RV64F-NEXT:#NO_APP
+; RV64F-NEXT:fmv.x.d a0, ft0
+; RV64F-NEXT:ret
+  %1 = load double, double* @gd
+  %2 = tail call double asm "fadd.d $0, $1, $2", "=f,f,f"(double %a, double %1)
+  ret double %2
+}
Index: llvm/trunk/test/CodeGen/RISCV/inline-asm-invalid.ll
===
--- llvm/trunk/test/CodeGen/RISCV/inline-asm-invalid.ll
+++ llvm/trunk/test/CodeGen/RISCV/inline-asm-invalid.ll
@@ -22,3 +22,11 @@
   tail call void asm sideeffect "csrwi mstatus, $0", "K"(i32 -1)
   ret void
 }
+
+define void @constraint_f() nounwind {
+; CHECK: error: couldn't allocate input reg for constraint 'f'
+  tail call void asm "fadd.s fa0, fa0, $0", "f"(float 0.0)
+; CHECK: error: couldn't allocate input reg for constraint 'f'
+  tail call void asm "fadd.d fa0, fa0, $0", "f"(double 0.0)
+  ret void
+}
Index: llvm/trunk/lib/Target/RISCV/RISCVISelLowering.cpp
===
--- llvm/trunk/lib/Target/RISCV/RISCVISelLowering.cpp
+++ llvm/trunk/lib/Target/RISCV/RISCVISelLowering.cpp
@@ -2397,6 +2397,21 @@
   return nullptr;
 }
 
+/// getConstraintType - Given a constraint letter, return the type of
+/// constraint it is for this target.
+RISCVTargetLowering::ConstraintType
+RISCVTargetLowering::getConstraintType(StringRef Constraint) const {
+  if (Constraint.size() == 1) {
+s

[PATCH] D65500: [RISCV] Support 'f' Inline Assembly Constraint

[Sam Elliott via Phabricator via cfe-commits]

lenary created this revision.
lenary added reviewers: asb, lewis-revill.
Herald added subscribers: llvm-commits, cfe-commits, s.egerton, Jim, benna, 
psnobl, jocewei, PkmX, rkruppe, the_o, brucehoult, MartinMosbeck, rogfer01, 
edward-jones, zzheng, MaskRay, jrtc27, shiva0217, kito-cheng, niosHD, sabuasal, 
apazos, simoncook, johnrusso, rbar, hiraditya.
Herald added projects: clang, LLVM.

This adds the 'f' inline assembly constraint, as supported by GCC. An
'f'-constrained operand is passed in a floating point register. Exactly
which kind of floating-point register (32-bit or 64-bit) is decided
based on the operand type and the available standard extensions (-f and
-d, respectively).

This patch adds support in both the clang frontend, and LLVM itself.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D65500

Files:
  clang/lib/Basic/Targets/RISCV.cpp
  clang/test/CodeGen/riscv-inline-asm.c
  llvm/lib/Target/RISCV/RISCVISelLowering.cpp
  llvm/lib/Target/RISCV/RISCVISelLowering.h
  llvm/test/CodeGen/RISCV/inline-asm-d-constraint-f.ll
  llvm/test/CodeGen/RISCV/inline-asm-f-constraint-f.ll
  llvm/test/CodeGen/RISCV/inline-asm-invalid.ll

Index: llvm/test/CodeGen/RISCV/inline-asm-invalid.ll
===
--- llvm/test/CodeGen/RISCV/inline-asm-invalid.ll
+++ llvm/test/CodeGen/RISCV/inline-asm-invalid.ll
@@ -22,3 +22,11 @@
   tail call void asm sideeffect "csrwi mstatus, $0", "K"(i32 -1)
   ret void
 }
+
+define void @constraint_f() nounwind {
+; CHECK: error: couldn't allocate output register for constraint 'f'
+  %1 = tail call float asm "fadd.s $0, $1, $2", "=f,f,f"(float 0.0, float 1.0)
+; CHECK: error: couldn't allocate output register for constraint 'f'
+  %2 = tail call double asm "fadd.d $0, $1, $2", "=f,f,f"(double 0.0, double 1.0)
+  ret void
+}
Index: llvm/test/CodeGen/RISCV/inline-asm-f-constraint-f.ll
===
--- /dev/null
+++ llvm/test/CodeGen/RISCV/inline-asm-f-constraint-f.ll
@@ -0,0 +1,34 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc -mtriple=riscv32 -mattr=+f -verify-machineinstrs < %s \
+; RUN:   | FileCheck -check-prefix=RV32F %s
+; RUN: llc -mtriple=riscv64 -mattr=+f -verify-machineinstrs < %s \
+; RUN:   | FileCheck -check-prefix=RV64F %s
+
+@gf = external global float
+
+define float @constraint_f_float(float %a) nounwind {
+; RV32F-LABEL: constraint_f_float:
+; RV32F:   # %bb.0:
+; RV32F-NEXT:fmv.w.x ft0, a0
+; RV32F-NEXT:lui a0, %hi(gf)
+; RV32F-NEXT:flw ft1, %lo(gf)(a0)
+; RV32F-NEXT:#APP
+; RV32F-NEXT:fadd.s ft0, ft0, ft1
+; RV32F-NEXT:#NO_APP
+; RV32F-NEXT:fmv.x.w a0, ft0
+; RV32F-NEXT:ret
+;
+; RV64F-LABEL: constraint_f_float:
+; RV64F:   # %bb.0:
+; RV64F-NEXT:fmv.w.x ft0, a0
+; RV64F-NEXT:lui a0, %hi(gf)
+; RV64F-NEXT:flw ft1, %lo(gf)(a0)
+; RV64F-NEXT:#APP
+; RV64F-NEXT:fadd.s ft0, ft0, ft1
+; RV64F-NEXT:#NO_APP
+; RV64F-NEXT:fmv.x.w a0, ft0
+; RV64F-NEXT:ret
+  %1 = load float, float* @gf
+  %2 = tail call float asm "fadd.s $0, $1, $2", "=f,f,f"(float %a, float %1)
+  ret float %2
+}
Index: llvm/test/CodeGen/RISCV/inline-asm-d-constraint-f.ll
===
--- /dev/null
+++ llvm/test/CodeGen/RISCV/inline-asm-d-constraint-f.ll
@@ -0,0 +1,40 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc -mtriple=riscv32 -mattr=+d -verify-machineinstrs < %s \
+; RUN:   | FileCheck -check-prefix=RV32F %s
+; RUN: llc -mtriple=riscv64 -mattr=+d -verify-machineinstrs < %s \
+; RUN:   | FileCheck -check-prefix=RV64F %s
+
+@gd = external global double
+
+define double @constraint_f_double(double %a) nounwind {
+; RV32F-LABEL: constraint_f_double:
+; RV32F:   # %bb.0:
+; RV32F-NEXT:addi sp, sp, -16
+; RV32F-NEXT:sw a0, 8(sp)
+; RV32F-NEXT:sw a1, 12(sp)
+; RV32F-NEXT:fld ft0, 8(sp)
+; RV32F-NEXT:lui a0, %hi(gd)
+; RV32F-NEXT:fld ft1, %lo(gd)(a0)
+; RV32F-NEXT:#APP
+; RV32F-NEXT:fadd.d ft0, ft0, ft1
+; RV32F-NEXT:#NO_APP
+; RV32F-NEXT:fsd ft0, 8(sp)
+; RV32F-NEXT:lw a0, 8(sp)
+; RV32F-NEXT:lw a1, 12(sp)
+; RV32F-NEXT:addi sp, sp, 16
+; RV32F-NEXT:ret
+;
+; RV64F-LABEL: constraint_f_double:
+; RV64F:   # %bb.0:
+; RV64F-NEXT:fmv.d.x ft0, a0
+; RV64F-NEXT:lui a0, %hi(gd)
+; RV64F-NEXT:fld ft1, %lo(gd)(a0)
+; RV64F-NEXT:#APP
+; RV64F-NEXT:fadd.d ft0, ft0, ft1
+; RV64F-NEXT:#NO_APP
+; RV64F-NEXT:fmv.x.d a0, ft0
+; RV64F-NEXT:ret
+  %1 = load double, double* @gd
+  %2 = tail call double asm "fadd.d $0, $1, $2", "=f,f,f"(double %a, double %1)
+  ret double %2
+}
Index: llvm/lib/Target/RISCV/RISCVISelLowering.h
===
--- llvm/lib/Target/RISCV/RISCVISelLowering.h
+++ llvm/lib/Target/RISCV/RISCVISelLowering.h
@@ -92,

[PATCH] D65500: [RISCV] Support 'f' Inline Assembly Constraint

[Sam Elliott via Phabricator via cfe-commits]

lenary updated this revision to Diff 212513.
lenary added a comment.

- Simplify inline-asm-invalid.ll test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65500

Files:
  clang/lib/Basic/Targets/RISCV.cpp
  clang/test/CodeGen/riscv-inline-asm.c
  llvm/lib/Target/RISCV/RISCVISelLowering.cpp
  llvm/lib/Target/RISCV/RISCVISelLowering.h
  llvm/test/CodeGen/RISCV/inline-asm-d-constraint-f.ll
  llvm/test/CodeGen/RISCV/inline-asm-f-constraint-f.ll
  llvm/test/CodeGen/RISCV/inline-asm-invalid.ll

Index: llvm/test/CodeGen/RISCV/inline-asm-invalid.ll
===
--- llvm/test/CodeGen/RISCV/inline-asm-invalid.ll
+++ llvm/test/CodeGen/RISCV/inline-asm-invalid.ll
@@ -22,3 +22,11 @@
   tail call void asm sideeffect "csrwi mstatus, $0", "K"(i32 -1)
   ret void
 }
+
+define void @constraint_f() nounwind {
+; CHECK: error: couldn't allocate input reg for constraint 'f'
+  tail call void asm "fadd.s fa0, fa0, $0", "f"(float 0.0)
+; CHECK: error: couldn't allocate input reg for constraint 'f'
+  tail call void asm "fadd.d fa0, fa0, $0", "f"(double 0.0)
+  ret void
+}
Index: llvm/test/CodeGen/RISCV/inline-asm-f-constraint-f.ll
===
--- /dev/null
+++ llvm/test/CodeGen/RISCV/inline-asm-f-constraint-f.ll
@@ -0,0 +1,34 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc -mtriple=riscv32 -mattr=+f -verify-machineinstrs < %s \
+; RUN:   | FileCheck -check-prefix=RV32F %s
+; RUN: llc -mtriple=riscv64 -mattr=+f -verify-machineinstrs < %s \
+; RUN:   | FileCheck -check-prefix=RV64F %s
+
+@gf = external global float
+
+define float @constraint_f_float(float %a) nounwind {
+; RV32F-LABEL: constraint_f_float:
+; RV32F:   # %bb.0:
+; RV32F-NEXT:fmv.w.x ft0, a0
+; RV32F-NEXT:lui a0, %hi(gf)
+; RV32F-NEXT:flw ft1, %lo(gf)(a0)
+; RV32F-NEXT:#APP
+; RV32F-NEXT:fadd.s ft0, ft0, ft1
+; RV32F-NEXT:#NO_APP
+; RV32F-NEXT:fmv.x.w a0, ft0
+; RV32F-NEXT:ret
+;
+; RV64F-LABEL: constraint_f_float:
+; RV64F:   # %bb.0:
+; RV64F-NEXT:fmv.w.x ft0, a0
+; RV64F-NEXT:lui a0, %hi(gf)
+; RV64F-NEXT:flw ft1, %lo(gf)(a0)
+; RV64F-NEXT:#APP
+; RV64F-NEXT:fadd.s ft0, ft0, ft1
+; RV64F-NEXT:#NO_APP
+; RV64F-NEXT:fmv.x.w a0, ft0
+; RV64F-NEXT:ret
+  %1 = load float, float* @gf
+  %2 = tail call float asm "fadd.s $0, $1, $2", "=f,f,f"(float %a, float %1)
+  ret float %2
+}
Index: llvm/test/CodeGen/RISCV/inline-asm-d-constraint-f.ll
===
--- /dev/null
+++ llvm/test/CodeGen/RISCV/inline-asm-d-constraint-f.ll
@@ -0,0 +1,40 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc -mtriple=riscv32 -mattr=+d -verify-machineinstrs < %s \
+; RUN:   | FileCheck -check-prefix=RV32F %s
+; RUN: llc -mtriple=riscv64 -mattr=+d -verify-machineinstrs < %s \
+; RUN:   | FileCheck -check-prefix=RV64F %s
+
+@gd = external global double
+
+define double @constraint_f_double(double %a) nounwind {
+; RV32F-LABEL: constraint_f_double:
+; RV32F:   # %bb.0:
+; RV32F-NEXT:addi sp, sp, -16
+; RV32F-NEXT:sw a0, 8(sp)
+; RV32F-NEXT:sw a1, 12(sp)
+; RV32F-NEXT:fld ft0, 8(sp)
+; RV32F-NEXT:lui a0, %hi(gd)
+; RV32F-NEXT:fld ft1, %lo(gd)(a0)
+; RV32F-NEXT:#APP
+; RV32F-NEXT:fadd.d ft0, ft0, ft1
+; RV32F-NEXT:#NO_APP
+; RV32F-NEXT:fsd ft0, 8(sp)
+; RV32F-NEXT:lw a0, 8(sp)
+; RV32F-NEXT:lw a1, 12(sp)
+; RV32F-NEXT:addi sp, sp, 16
+; RV32F-NEXT:ret
+;
+; RV64F-LABEL: constraint_f_double:
+; RV64F:   # %bb.0:
+; RV64F-NEXT:fmv.d.x ft0, a0
+; RV64F-NEXT:lui a0, %hi(gd)
+; RV64F-NEXT:fld ft1, %lo(gd)(a0)
+; RV64F-NEXT:#APP
+; RV64F-NEXT:fadd.d ft0, ft0, ft1
+; RV64F-NEXT:#NO_APP
+; RV64F-NEXT:fmv.x.d a0, ft0
+; RV64F-NEXT:ret
+  %1 = load double, double* @gd
+  %2 = tail call double asm "fadd.d $0, $1, $2", "=f,f,f"(double %a, double %1)
+  ret double %2
+}
Index: llvm/lib/Target/RISCV/RISCVISelLowering.h
===
--- llvm/lib/Target/RISCV/RISCVISelLowering.h
+++ llvm/lib/Target/RISCV/RISCVISelLowering.h
@@ -92,6 +92,7 @@
   // This method returns the name of a target specific DAG node.
   const char *getTargetNodeName(unsigned Opcode) const override;
 
+  ConstraintType getConstraintType(StringRef Constraint) const override;
   std::pair
   getRegForInlineAsmConstraint(const TargetRegisterInfo *TRI,
StringRef Constraint, MVT VT) const override;
Index: llvm/lib/Target/RISCV/RISCVISelLowering.cpp
===
--- llvm/lib/Target/RISCV/RISCVISelLowering.cpp
+++ llvm/lib/Target/RISCV/RISCVISelLowering.cpp
@@ -2397,6 +2397,21 @@
   return nullptr;
 }
 
+/// get

[PATCH] D57795: [RISCV] Add FreeBSD targets

[Sam Elliott via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL367557: [RISCV] Add FreeBSD targets (authored by lenary, 
committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D57795?vs=203359&id=212795#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D57795

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


Index: cfe/trunk/lib/Basic/Targets.cpp
===
--- cfe/trunk/lib/Basic/Targets.cpp
+++ cfe/trunk/lib/Basic/Targets.cpp
@@ -363,15 +363,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: cfe/trunk/lib/Driver/ToolChains/FreeBSD.cpp
===
--- cfe/trunk/lib/Driver/ToolChains/FreeBSD.cpp
+++ cfe/trunk/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: cfe/trunk/test/Driver/freebsd.c
===
--- cfe/trunk/test/Driver/freebsd.c
+++ cfe/trunk/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: cfe/trunk/lib/Basic/Targets.cpp
===
--- cfe/trunk/lib/Basic/Targets.cpp
+++ cfe/trunk/lib/Basic/Targets.cpp
@@ -363,15 +363,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: cfe/trunk/lib/Driver/ToolChains/FreeBSD.cpp
===
--- cfe/trunk/lib/Driver/ToolChains/FreeBSD.cpp
+++ cfe/trunk/lib/Driver/ToolChains/FreeBSD.cpp
@@ -197,6 +197,14 @@
 else
   CmdArgs.push_back("elf64ltsmip_fbsd");
 break;
+  case llvm::Triple::riscv32:
+CmdArgs.push_back("-m");
+ 

[PATCH] D48357: [RISCV] Remove duplicated logic when determining the target ABI

[Sam Elliott via Phabricator via cfe-commits]

lenary added a comment.
Herald added subscribers: s.egerton, Jim, benna, psnobl, MaskRay.

Given we now have support for the floating-point ABIs, can you rebase and 
update this patch?


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

https://reviews.llvm.org/D48357



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

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

[Sam Elliott via Phabricator via cfe-commits]

lenary added a comment.

What happens if I pass `clang -march=rv32i -target riscv64-unknown-elf`? Should 
we care about the ordering of `-march` vs `-target`?


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] D63497: Add support for openSUSE RISC-V triple

[Sam Elliott via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL367565: Add support for openSUSE RISC-V triple (authored by 
lenary, committed by ).
Herald added a subscriber: s.egerton.

Changed prior to commit:
  https://reviews.llvm.org/D63497?vs=206423&id=212814#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D63497

Files:
  cfe/trunk/lib/Driver/ToolChains/Gnu.cpp
  cfe/trunk/test/Driver/Inputs/opensuse_tumbleweed_riscv64_tree/usr/lib64/crt1.o
  cfe/trunk/test/Driver/Inputs/opensuse_tumbleweed_riscv64_tree/usr/lib64/crti.o
  cfe/trunk/test/Driver/Inputs/opensuse_tumbleweed_riscv64_tree/usr/lib64/crtn.o
  
cfe/trunk/test/Driver/Inputs/opensuse_tumbleweed_riscv64_tree/usr/lib64/gcc/riscv64-suse-linux/9/crtbegin.o
  
cfe/trunk/test/Driver/Inputs/opensuse_tumbleweed_riscv64_tree/usr/lib64/gcc/riscv64-suse-linux/9/crtend.o
  cfe/trunk/test/Driver/linux-ld.c
  llvm/trunk/unittests/ADT/TripleTest.cpp


Index: llvm/trunk/unittests/ADT/TripleTest.cpp
===
--- llvm/trunk/unittests/ADT/TripleTest.cpp
+++ llvm/trunk/unittests/ADT/TripleTest.cpp
@@ -330,6 +330,12 @@
   EXPECT_EQ(Triple::FreeBSD, T.getOS());
   EXPECT_EQ(Triple::UnknownEnvironment, T.getEnvironment());
 
+  T = Triple("riscv64-suse-linux");
+  EXPECT_EQ(Triple::riscv64, T.getArch());
+  EXPECT_EQ(Triple::SUSE, T.getVendor());
+  EXPECT_EQ(Triple::Linux, T.getOS());
+  EXPECT_EQ(Triple::UnknownEnvironment, T.getEnvironment());
+
   T = Triple("armv7hl-suse-linux-gnueabi");
   EXPECT_EQ(Triple::arm, T.getArch());
   EXPECT_EQ(Triple::SUSE, T.getVendor());
Index: cfe/trunk/lib/Driver/ToolChains/Gnu.cpp
===
--- cfe/trunk/lib/Driver/ToolChains/Gnu.cpp
+++ cfe/trunk/lib/Driver/ToolChains/Gnu.cpp
@@ -2017,7 +2017,8 @@
   static const char *const RISCV64LibDirs[] = {"/lib64", "/lib"};
   static const char *const RISCV64Triples[] = {"riscv64-unknown-linux-gnu",
"riscv64-linux-gnu",
-   "riscv64-unknown-elf"};
+   "riscv64-unknown-elf",
+   "riscv64-suse-linux"};
 
   static const char *const SPARCv8LibDirs[] = {"/lib32", "/lib"};
   static const char *const SPARCv8Triples[] = {"sparc-linux-gnu",
Index: cfe/trunk/test/Driver/linux-ld.c
===
--- cfe/trunk/test/Driver/linux-ld.c
+++ cfe/trunk/test/Driver/linux-ld.c
@@ -859,6 +859,26 @@
 // CHECK-OPENSUSE-TW-ARMV7HL: 
"{{.*}}/usr/lib/gcc/armv7hl-suse-linux-gnueabi/5{{/|}}crtend.o"
 // CHECK-OPENSUSE-TW-ARMV7HL: 
"{{.*}}/usr/lib/gcc/armv7hl-suse-linux-gnueabi/5/../../../../lib{{/|}}crtn.o"
 //
+// Check openSUSE Tumbleweed on riscv64
+// RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
+// RUN: --target=riscv64-suse-linux -rtlib=platform \
+// RUN: --gcc-toolchain="" \
+// RUN: --sysroot=%S/Inputs/opensuse_tumbleweed_riscv64_tree \
+// RUN:   | FileCheck --check-prefix=CHECK-OPENSUSE-TW-RISCV64 %s
+// RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
+// RUN: --target=riscv64-suse-linux -rtlib=platform \
+// RUN: --gcc-toolchain="" \
+// RUN: --sysroot=%S/Inputs/opensuse_tumbleweed_riscv64_tree \
+// RUN:   | FileCheck --check-prefix=CHECK-OPENSUSE-TW-RISCV64 %s
+// CHECK-OPENSUSE-TW-RISCV64: "{{.*}}ld{{(.exe)?}}" 
"--sysroot=[[SYSROOT:[^"]+]]"
+// CHECK-OPENSUSE-TW-RISCV64: 
"{{.*}}/usr/lib64/gcc/riscv64-suse-linux/9/../../../../lib64{{/|}}crt1.o"
+// CHECK-OPENSUSE-TW-RISCV64: 
"{{.*}}/usr/lib64/gcc/riscv64-suse-linux/9/../../../../lib64{{/|}}crti.o"
+// CHECK-OPENSUSE-TW-RISCV64: 
"{{.*}}/usr/lib64/gcc/riscv64-suse-linux/9{{/|}}crtbegin.o"
+// CHECK-OPENSUSE-TW-RISCV64: 
"-L[[SYSROOT]]/usr/lib64/gcc/riscv64-suse-linux/9"
+// CHECK-OPENSUSE-TW-RISCV64: 
"-L[[SYSROOT]]/usr/lib64/gcc/riscv64-suse-linux/9/../../../../lib64"
+// CHECK-OPENSUSE-TW-RISCV64: 
"{{.*}}/usr/lib64/gcc/riscv64-suse-linux/9{{/|}}crtend.o"
+// CHECK-OPENSUSE-TW-RISCV64: 
"{{.*}}/usr/lib64/gcc/riscv64-suse-linux/9/../../../../lib64{{/|}}crtn.o"
+//
 // Check dynamic-linker for different archs
 // RUN: %clang %s -### -o %t.o 2>&1 \
 // RUN: --target=arm-linux-gnueabi \


Index: llvm/trunk/unittests/ADT/TripleTest.cpp
===
--- llvm/trunk/unittests/ADT/TripleTest.cpp
+++ llvm/trunk/unittests/ADT/TripleTest.cpp
@@ -330,6 +330,12 @@
   EXPECT_EQ(Triple::FreeBSD, T.getOS());
   EXPECT_EQ(Triple::UnknownEnvironment, T.getEnvironment());
 
+  T = Triple("riscv64-suse-linux");
+  EXPECT_EQ(Triple::riscv64, T.getArch());
+  EXPECT_EQ(Triple::SUSE, T.getVendor());
+  EXPECT_EQ(Triple::Linux, T.getOS());
+  EXPECT_EQ(Triple::UnknownEnvironment, T.getEnvironme

[PATCH] D63497: Add support for openSUSE RISC-V triple

[Sam Elliott via Phabricator via cfe-commits]

lenary added a comment.

Thanks @schwab, sorry we took so long to merge your patch, but it's merged now!


Repository:
  rL LLVM

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

https://reviews.llvm.org/D63497



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

[PATCH] D48357: [RISCV] Remove duplicated logic when determining the target ABI

[Sam Elliott via Phabricator via cfe-commits]

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

Ok, sure! I think I'm happy for this to land then.


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

https://reviews.llvm.org/D48357



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

[PATCH] D65634: [RISCV] Default to lp64d in 64-bit RISC-V Linux

[Sam Elliott via Phabricator via cfe-commits]

lenary added inline comments.



Comment at: clang/lib/Driver/ToolChains/Arch/RISCV.cpp:385
+ ? "ilp32"
+ : Triple.getOS() == llvm::Triple::Linux ? "lp64d" : "lp64";
 }

Please may you turn this into a set of if-statements? Nesting ternary operators 
is a recipe for confusion.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65634



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

[PATCH] D66266: [WIP][RISCV] Set MaxAtomicPromoteWidth and MaxAtomicInlineWidth

[Sam Elliott via Phabricator via cfe-commits]

lenary added a comment.

Hi Pengxuan,

Thanks for the patch! We have a similar patch from @asb under review at the 
moment, D57450 . There's one question about 
ABI compatibility that still needs to be answered, which we will discuss in the 
RISC-V sync-up later today.

Sam


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66266



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

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

[Sam Elliott via Phabricator via cfe-commits]

lenary added a comment.
Herald added subscribers: s.egerton, MaskRay.

Given this is an ABI-compatibility issue, I've been looking at how GCC and 
Clang differ in how they deal with issues around size and alignment of atomic 
objects.

All types of size less than or equal to `MaxAtomicPromoteWidth`, when they are 
turned into their atomic variant, will: a) have their size rounded up to a 
power of two, and b) have their alignment changed to that same power of two.

The following Gist contains:

- a C program which will print the size and alignment of a variety of strangely 
sized and aligned objects, and their atomic variants.
- the output of this program when run through gcc and clang, specifying 
`rv(32,64)ima(,fd)` and `(ilp32,lp64)(,f)` (matching ABIs to architectures). I 
used `riscv64-unknown-linux-gnu-gcc` for the avoidance of doubt (this compiler 
allows you to compile for rv32 by specifying a 32-bit -march value).
- the diffs in those outputs for an single ABI, between the two different 
compilers.

The gist is here: 
https://gist.github.com/lenary/2e977a8af33876ba8e0605e98c4e1b0d

There are some differences between GCC and Clang, that seem not to be risc-v 
specific (I saw them when running the C program on my mac)

- Clang ensures zero-sized objects become char-sized when they become atomic. 
GCC leaves them zero-sized. This is documented in ASTContext.cpp line 2130, to 
ensure the atomic operation is generated.
- GCC seems not to round up the size and alignment of non-power-of-two-sized 
structures.

I think this patch needs to be updated to ensure there are no differences in 
objects of power-of-two size. To do this, both riscv64 and riscv32, should have 
a `MaxAtomicPromoteWidth` of 128.

@jyknight does this reasoning make sense for ABI compatibility?


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] D57450: [RISCV] Set MaxAtomicInlineWidth and MaxAtomicPromoteWidth for RV32/RV64 targets with atomics

[Sam Elliott via Phabricator via cfe-commits]

lenary commandeered this revision.
lenary added a reviewer: asb.
lenary added a comment.

Chatted to @asb and he wants me to take over this set of changes.


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] D57450: [RISCV] Set MaxAtomicInlineWidth and MaxAtomicPromoteWidth for RV32/RV64 targets with atomics

[Sam Elliott via Phabricator via cfe-commits]

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

Upon further thought, I realise that `MaxAtomicPromoteWidth` should be set to 
128 regardless of whether a target `HasA`. I will be updating the patch with 
the new width and conditions early next week.


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] D63498: [RISC-V] Add -msave-restore and -mno-save-restore to clang driver

[Sam Elliott via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL364018: [RISC-V] Add -msave-restore and -mno-save-restore to 
clang driver (authored by lenary, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D63498?vs=205846&id=205955#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D63498

Files:
  cfe/trunk/include/clang/Driver/Options.td
  cfe/trunk/lib/Driver/ToolChains/Arch/RISCV.cpp
  cfe/trunk/test/Driver/riscv-features.c


Index: cfe/trunk/lib/Driver/ToolChains/Arch/RISCV.cpp
===
--- cfe/trunk/lib/Driver/ToolChains/Arch/RISCV.cpp
+++ cfe/trunk/lib/Driver/ToolChains/Arch/RISCV.cpp
@@ -358,6 +358,16 @@
   else
 Features.push_back("-relax");
 
+  // -mno-save-restore is default, unless -msave-restore is specified.
+  if (Args.hasFlag(options::OPT_msave_restore, options::OPT_mno_save_restore, 
false)) {
+Features.push_back("+save-restore");
+// ... but we don't yet support +save-restore, so issue a warning.
+D.Diag(diag::warn_drv_clang_unsupported)
+  << Args.getLastArg(options::OPT_msave_restore)->getAsString(Args);
+  } else {
+Features.push_back("-save-restore");
+  }
+
   // Now add any that the user explicitly requested on the command line,
   // which may override the defaults.
   handleTargetFeaturesGroup(Args, Features, 
options::OPT_m_riscv_Features_Group);
Index: cfe/trunk/include/clang/Driver/Options.td
===
--- cfe/trunk/include/clang/Driver/Options.td
+++ cfe/trunk/include/clang/Driver/Options.td
@@ -2130,6 +2130,10 @@
   HelpText<"Enable linker relaxation">;
 def mno_relax : Flag<["-"], "mno-relax">, Group,
   HelpText<"Disable linker relaxation">;
+def msave_restore : Flag<["-"], "msave-restore">, 
Group,
+  HelpText<"Enable using library calls for save and restore">;
+def mno_save_restore : Flag<["-"], "mno-save-restore">, 
Group,
+  HelpText<"Disable using library calls for save and restore">;
 
 def munaligned_access : Flag<["-"], "munaligned-access">, 
Group,
   HelpText<"Allow memory accesses to be unaligned (AArch32/AArch64 only)">;
Index: cfe/trunk/test/Driver/riscv-features.c
===
--- cfe/trunk/test/Driver/riscv-features.c
+++ cfe/trunk/test/Driver/riscv-features.c
@@ -3,11 +3,23 @@
 
 // CHECK: fno-signed-char
 
+// RUN: %clang -target riscv32-unknown-elf -### %s 2>&1 | FileCheck %s 
-check-prefix=DEFAULT
+
 // RUN: %clang -target riscv32-unknown-elf -### %s -mrelax 2>&1 | FileCheck %s 
-check-prefix=RELAX
 // RUN: %clang -target riscv32-unknown-elf -### %s -mno-relax 2>&1 | FileCheck 
%s -check-prefix=NO-RELAX
-// RUN: %clang -target riscv32-unknown-elf -### %s 2>&1 | FileCheck %s 
-check-prefix=DEFAULT
 
 // RELAX: "-target-feature" "+relax"
 // NO-RELAX: "-target-feature" "-relax"
 // DEFAULT: "-target-feature" "+relax"
 // DEFAULT-NOT: "-target-feature" "-relax"
+
+// RUN: %clang -target riscv32-unknown-elf -### %s -msave-restore 2>&1 | 
FileCheck %s -check-prefix=SAVE-RESTORE
+// RUN: %clang -target riscv32-unknown-elf -### %s -mno-save-restore 2>&1 | 
FileCheck %s -check-prefix=NO-SAVE-RESTORE
+
+// SAVE-RESTORE: warning: the clang compiler does not support '-msave-restore'
+// DEFAULT-NOT: warning: the clang compiler does not support
+
+// SAVE-RESTORE: "-target-feature" "+save-restore"
+// NO-SAVE-RESTORE: "-target-feature" "-save-restore"
+// DEFAULT: "-target-feature" "-save-restore"
+// DEFAULT-NOT: "-target-feature" "+save-restore"
\ No newline at end of file


Index: cfe/trunk/lib/Driver/ToolChains/Arch/RISCV.cpp
===
--- cfe/trunk/lib/Driver/ToolChains/Arch/RISCV.cpp
+++ cfe/trunk/lib/Driver/ToolChains/Arch/RISCV.cpp
@@ -358,6 +358,16 @@
   else
 Features.push_back("-relax");
 
+  // -mno-save-restore is default, unless -msave-restore is specified.
+  if (Args.hasFlag(options::OPT_msave_restore, options::OPT_mno_save_restore, false)) {
+Features.push_back("+save-restore");
+// ... but we don't yet support +save-restore, so issue a warning.
+D.Diag(diag::warn_drv_clang_unsupported)
+  << Args.getLastArg(options::OPT_msave_restore)->getAsString(Args);
+  } else {
+Features.push_back("-save-restore");
+  }
+
   // Now add any that the user explicitly requested on the command line,
   // which may override the defaults.
   handleTargetFeaturesGroup(Args, Features, options::OPT_m_riscv_Features_Group);
Index: cfe/trunk/include/clang/Driver/Options.td
===
--- cfe/trunk/include/clang/Driver/Options.td
+++ cfe/trunk/include/clang/Driver/Options.td
@@ -2130,6 +2130,10 @@
   HelpText<"Enable linker relaxation">;
 def mno_relax : 

[PATCH] D64008: [RISCV] Avoid save-restore target feature warning

[Sam Elliott via Phabricator via cfe-commits]

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

LLVM issues a warning if passed unknown target features. Neither I nor
@asb noticed this until after https://reviews.llvm.org/D63498 landed.

This patch stops passing the (unknown) "save-restore" target feature to
the LLVM backend, but continues to emit a warning if a driver asks for
`-msave-restore`. The default of assuming `-mno-save-restore` (and
emitting no warnings) remains.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D64008

Files:
  clang/lib/Driver/ToolChains/Arch/RISCV.cpp
  clang/test/Driver/riscv-features.c


Index: clang/test/Driver/riscv-features.c
===
--- clang/test/Driver/riscv-features.c
+++ clang/test/Driver/riscv-features.c
@@ -17,9 +17,5 @@
 // RUN: %clang -target riscv32-unknown-elf -### %s -mno-save-restore 2>&1 | 
FileCheck %s -check-prefix=NO-SAVE-RESTORE
 
 // SAVE-RESTORE: warning: the clang compiler does not support '-msave-restore'
-// DEFAULT-NOT: warning: the clang compiler does not support
-
-// SAVE-RESTORE: "-target-feature" "+save-restore"
-// NO-SAVE-RESTORE: "-target-feature" "-save-restore"
-// DEFAULT: "-target-feature" "-save-restore"
-// DEFAULT-NOT: "-target-feature" "+save-restore"
\ No newline at end of file
+// NO-SAVE-RESTORE-NOT: warning: the clang compiler does not support
+// DEFAULT-NOT: warning: the clang compiler does not support
\ No newline at end of file
Index: clang/lib/Driver/ToolChains/Arch/RISCV.cpp
===
--- clang/lib/Driver/ToolChains/Arch/RISCV.cpp
+++ clang/lib/Driver/ToolChains/Arch/RISCV.cpp
@@ -358,14 +358,12 @@
   else
 Features.push_back("-relax");
 
-  // -mno-save-restore is default, unless -msave-restore is specified.
+  // GCC Compatibility: -mno-save-restore is default, unless -msave-restore is
+  // specified...
   if (Args.hasFlag(options::OPT_msave_restore, options::OPT_mno_save_restore, 
false)) {
-Features.push_back("+save-restore");
-// ... but we don't yet support +save-restore, so issue a warning.
+// ... but we don't support -msave-restore, so issue a warning.
 D.Diag(diag::warn_drv_clang_unsupported)
   << Args.getLastArg(options::OPT_msave_restore)->getAsString(Args);
-  } else {
-Features.push_back("-save-restore");
   }
 
   // Now add any that the user explicitly requested on the command line,


Index: clang/test/Driver/riscv-features.c
===
--- clang/test/Driver/riscv-features.c
+++ clang/test/Driver/riscv-features.c
@@ -17,9 +17,5 @@
 // RUN: %clang -target riscv32-unknown-elf -### %s -mno-save-restore 2>&1 | FileCheck %s -check-prefix=NO-SAVE-RESTORE
 
 // SAVE-RESTORE: warning: the clang compiler does not support '-msave-restore'
-// DEFAULT-NOT: warning: the clang compiler does not support
-
-// SAVE-RESTORE: "-target-feature" "+save-restore"
-// NO-SAVE-RESTORE: "-target-feature" "-save-restore"
-// DEFAULT: "-target-feature" "-save-restore"
-// DEFAULT-NOT: "-target-feature" "+save-restore"
\ No newline at end of file
+// NO-SAVE-RESTORE-NOT: warning: the clang compiler does not support
+// DEFAULT-NOT: warning: the clang compiler does not support
\ No newline at end of file
Index: clang/lib/Driver/ToolChains/Arch/RISCV.cpp
===
--- clang/lib/Driver/ToolChains/Arch/RISCV.cpp
+++ clang/lib/Driver/ToolChains/Arch/RISCV.cpp
@@ -358,14 +358,12 @@
   else
 Features.push_back("-relax");
 
-  // -mno-save-restore is default, unless -msave-restore is specified.
+  // GCC Compatibility: -mno-save-restore is default, unless -msave-restore is
+  // specified...
   if (Args.hasFlag(options::OPT_msave_restore, options::OPT_mno_save_restore, false)) {
-Features.push_back("+save-restore");
-// ... but we don't yet support +save-restore, so issue a warning.
+// ... but we don't support -msave-restore, so issue a warning.
 D.Diag(diag::warn_drv_clang_unsupported)
   << Args.getLastArg(options::OPT_msave_restore)->getAsString(Args);
-  } else {
-Features.push_back("-save-restore");
   }
 
   // Now add any that the user explicitly requested on the command line,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D64008: [RISCV] Avoid save-restore target feature warning

[Sam Elliott via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL364777: [RISCV] Avoid save-restore target feature warning 
(authored by lenary, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D64008?vs=207295&id=207309#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D64008

Files:
  cfe/trunk/lib/Driver/ToolChains/Arch/RISCV.cpp
  cfe/trunk/test/Driver/riscv-features.c


Index: cfe/trunk/lib/Driver/ToolChains/Arch/RISCV.cpp
===
--- cfe/trunk/lib/Driver/ToolChains/Arch/RISCV.cpp
+++ cfe/trunk/lib/Driver/ToolChains/Arch/RISCV.cpp
@@ -358,14 +358,12 @@
   else
 Features.push_back("-relax");
 
-  // -mno-save-restore is default, unless -msave-restore is specified.
+  // GCC Compatibility: -mno-save-restore is default, unless -msave-restore is
+  // specified...
   if (Args.hasFlag(options::OPT_msave_restore, options::OPT_mno_save_restore, 
false)) {
-Features.push_back("+save-restore");
-// ... but we don't yet support +save-restore, so issue a warning.
+// ... but we don't support -msave-restore, so issue a warning.
 D.Diag(diag::warn_drv_clang_unsupported)
   << Args.getLastArg(options::OPT_msave_restore)->getAsString(Args);
-  } else {
-Features.push_back("-save-restore");
   }
 
   // Now add any that the user explicitly requested on the command line,
Index: cfe/trunk/test/Driver/riscv-features.c
===
--- cfe/trunk/test/Driver/riscv-features.c
+++ cfe/trunk/test/Driver/riscv-features.c
@@ -17,9 +17,5 @@
 // RUN: %clang -target riscv32-unknown-elf -### %s -mno-save-restore 2>&1 | 
FileCheck %s -check-prefix=NO-SAVE-RESTORE
 
 // SAVE-RESTORE: warning: the clang compiler does not support '-msave-restore'
-// DEFAULT-NOT: warning: the clang compiler does not support
-
-// SAVE-RESTORE: "-target-feature" "+save-restore"
-// NO-SAVE-RESTORE: "-target-feature" "-save-restore"
-// DEFAULT: "-target-feature" "-save-restore"
-// DEFAULT-NOT: "-target-feature" "+save-restore"
\ No newline at end of file
+// NO-SAVE-RESTORE-NOT: warning: the clang compiler does not support
+// DEFAULT-NOT: warning: the clang compiler does not support
\ No newline at end of file


Index: cfe/trunk/lib/Driver/ToolChains/Arch/RISCV.cpp
===
--- cfe/trunk/lib/Driver/ToolChains/Arch/RISCV.cpp
+++ cfe/trunk/lib/Driver/ToolChains/Arch/RISCV.cpp
@@ -358,14 +358,12 @@
   else
 Features.push_back("-relax");
 
-  // -mno-save-restore is default, unless -msave-restore is specified.
+  // GCC Compatibility: -mno-save-restore is default, unless -msave-restore is
+  // specified...
   if (Args.hasFlag(options::OPT_msave_restore, options::OPT_mno_save_restore, false)) {
-Features.push_back("+save-restore");
-// ... but we don't yet support +save-restore, so issue a warning.
+// ... but we don't support -msave-restore, so issue a warning.
 D.Diag(diag::warn_drv_clang_unsupported)
   << Args.getLastArg(options::OPT_msave_restore)->getAsString(Args);
-  } else {
-Features.push_back("-save-restore");
   }
 
   // Now add any that the user explicitly requested on the command line,
Index: cfe/trunk/test/Driver/riscv-features.c
===
--- cfe/trunk/test/Driver/riscv-features.c
+++ cfe/trunk/test/Driver/riscv-features.c
@@ -17,9 +17,5 @@
 // RUN: %clang -target riscv32-unknown-elf -### %s -mno-save-restore 2>&1 | FileCheck %s -check-prefix=NO-SAVE-RESTORE
 
 // SAVE-RESTORE: warning: the clang compiler does not support '-msave-restore'
-// DEFAULT-NOT: warning: the clang compiler does not support
-
-// SAVE-RESTORE: "-target-feature" "+save-restore"
-// NO-SAVE-RESTORE: "-target-feature" "-save-restore"
-// DEFAULT: "-target-feature" "-save-restore"
-// DEFAULT-NOT: "-target-feature" "+save-restore"
\ No newline at end of file
+// NO-SAVE-RESTORE-NOT: warning: the clang compiler does not support
+// DEFAULT-NOT: warning: the clang compiler does not support
\ No newline at end of file
___
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

[Sam Elliott via Phabricator via cfe-commits]

lenary added a subscriber: luismarques.
lenary added a comment.

Nice, I like this new approach! One naming nit, but overall I think this is 
much better than the first version of the patch.

LGTM but I would like @luismarques to take a look too.




Comment at: llvm/lib/Target/RISCV/RISCVSubtarget.h:49
   RISCVABI::ABI TargetABI = RISCVABI::ABI_Unknown;
+  BitVector ReserveRegister;
   RISCVFrameLowering FrameLowering;

Nit: Can this have a name that reveals it is for registers that are reserved by 
the user, rather than generally reserved? 

We would like to differentiate `ra` being reserved because of the calling 
convention, vs `ra` being reserved because a user asked for `-ffixed-x1`.


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] D67065: [RISCV] Define __riscv_cmodel_medlow and __riscv_cmodel_medany correctly

[Sam Elliott via Phabricator via cfe-commits]

lenary added a comment.

LGTM, now I've looked at how LLVM itself supports code models. I don't mind if 
that TODO is or isn't deleted.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67065



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

[PATCH] D67409: [RISCV] enable LTO support, pass some options to linker.

[Sam Elliott via Phabricator via cfe-commits]

lenary added a comment.

Can you add a test for `riscv64-unknown-linux-gnu`? I think RISCVToolchain.cpp 
is only used by the `riscv64-unknown-elf` target, but I could be wrong.


Repository:
  rC Clang

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

https://reviews.llvm.org/D67409



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

[PATCH] D67066: [RISCV] Add option aliases: -mcmodel=medany and -mcmodel=medlow

[Sam Elliott via Phabricator via cfe-commits]

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

Looks good to me!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67066



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

[PATCH] D67409: [RISCV] enable LTO support, pass some options to linker.

[Sam Elliott via Phabricator via cfe-commits]

lenary added a comment.

I have some nits about explicit comments for arguments and default argument 
values for backwards compatibility. Other than that, it looks like a nice code 
cleanup.




Comment at: clang/lib/Driver/ToolChains/Clang.cpp:6071
   // Add the target features
-  getTargetFeatures(getToolChain(), Triple, Args, CmdArgs, true);
+  getTargetFeatures(getToolChain(), Triple, Args, CmdArgs, true, false);
 

This needs `/*ForAS=*/true, /*ForLTOPlugin=*/false)` so it's clear what the 
booleans are for. Same for other calls to `getTargetFeatures`.



Comment at: clang/lib/Driver/ToolChains/CommonArgs.h:134
+   llvm::opt::ArgStringList &CmdArgs, bool ForAS,
+   bool ForLTOPlugin);
+

The final argument here should probably default to `false`, so that out-of-tree 
backends do not break immediately when this patch lands.


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

https://reviews.llvm.org/D67409



___
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

[Sam Elliott via Phabricator via cfe-commits]

lenary added a comment.

We discussed this in the RISC-V meeting on 19 Sept 2019. @apazos says there are 
some SPEC failures in both 2006 and 2017, which would be good to triage.


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] D62686: [RISCV] Add support for save/restore of callee-saved registers via libcalls

[Sam Elliott via Phabricator via cfe-commits]

lenary added a comment.

Two nits, that I wanted to submit before the meeting, but didn't get around to.




Comment at: llvm/lib/Target/RISCV/RISCV.td:72
 
+def FeatureSaveRestore : SubtargetFeature<"save-restore", "EnableSaveRestore",
+  "true", "Enable save/restore.">;

Given the clang option defaults to false, I think it should here too, to avoid 
confusion in other frontends.



Comment at: llvm/lib/Target/RISCV/RISCVFrameLowering.cpp:278
   // Add CFI directives for callee-saved registers.
-  const std::vector &CSI = MFI.getCalleeSavedInfo();
-  // Iterate over list of callee-saved registers and emit .cfi_restore
-  // directives.
-  for (const auto &Entry : CSI) {
-Register Reg = Entry.getReg();
-unsigned CFIIndex = MF.addFrameInst(MCCFIInstruction::createRestore(
-nullptr, RI->getDwarfRegNum(Reg, true)));
-BuildMI(MBB, MBBI, DL, TII->get(TargetOpcode::CFI_INSTRUCTION))
-.addCFIIndex(CFIIndex);
+  if (!CSI.empty()) {
+// Iterate over list of callee-saved registers and emit .cfi_restore

Nit: You shouldn't need this `if` statement - the for loop just won't execute 
if CSI is empty, surely.


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] D62686: [RISCV] Add support for save/restore of callee-saved registers via libcalls

[Sam Elliott via Phabricator via cfe-commits]

lenary added inline comments.



Comment at: llvm/lib/Target/RISCV/RISCV.td:72
 
+def FeatureSaveRestore : SubtargetFeature<"save-restore", "EnableSaveRestore",
+  "true", "Enable save/restore.">;

lewis-revill wrote:
> lenary wrote:
> > Given the clang option defaults to false, I think it should here too, to 
> > avoid confusion in other frontends.
> This is simply a case of a confusing parameter of the SubtargetFeature class.
> 
> SubtargetFeature interprets this "true" string as 'Value the attribute to be 
> set to by feature'. IE: when the feature is enabled, what value should the 
> corresponding RISCVSubtarget variable be set to? Rather than a default value 
> for that variable.
Ah, apologies for the confusion.


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] D68362: [libunwind][RISCV] Add 64-bit RISC-V support

[Sam Elliott via Phabricator via cfe-commits]

lenary added a comment.

Nice! Thanks for adding support for RISC-V. I like the use of the ABI register 
names rather than the numeric names.

I have a few queries/nits, below.




Comment at: libunwind/include/libunwind.h:835
 
+// 64-bit RISC-V registers
+enum {

Please include a link to the RISC-V ELF psABI spec which defines these register 
numbers.



Comment at: libunwind/src/Registers.hpp:3553
+
+  GPRs_registers;
+  double  _floats[32];

Why is this a special GPR struct type, but the FPRs are not?



Comment at: libunwind/src/Registers.hpp:3585
+
+inline uint64_t Registers_riscv::getRegister(int regNum) const {
+  if (regNum == UNW_REG_IP)

Do you want to include an override in this function for `regNum == 
UNW_RISCV_X0` to always return zero?

The reason I ask is because I worry that 
`Registers_riscv::Registers_riscv(const void *registers)` could initialise a 
non-zero value into `_registers.__x[0]`, which could lead to really confusing 
bugs.

It might also make sense to include `assert(validRegister(regNum));` in both 
this function and `setRegister` as you've done with the float registers.



Comment at: libunwind/src/UnwindCursor.hpp:999
+#if defined (_LIBUNWIND_TARGET_RISCV)
+  int stepWithCompactEncoding(Registers_riscv &) {
+return UNW_EINVAL;

Nit: This should be formatted like the version for `Registers_sparc` above.



Comment at: libunwind/src/UnwindCursor.hpp:1071
+#if defined (_LIBUNWIND_TARGET_RISCV)
+  bool compactSaysUseDwarf(Registers_riscv &, uint32_t *) const {
+return true;

Nit: This should be formatted like the version for `Registers_sparc` above.



Comment at: libunwind/src/UnwindCursor.hpp:1143
+#if defined (_LIBUNWIND_TARGET_RISCV)
+  compact_unwind_encoding_t dwarfEncoding(Registers_riscv &) const {
+return 0;

Nit: This should be formatted like the version for `Registers_sparc` above.


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

https://reviews.llvm.org/D68362



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

[PATCH] D67409: [RISCV] enable LTO support, pass some options to linker.

[Sam Elliott via Phabricator via cfe-commits]

lenary accepted this revision.
lenary added a comment.
This revision is now accepted and ready to land.
Herald added a subscriber: hiraditya.

Nice, LGTM


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

https://reviews.llvm.org/D67409



___
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

[Sam Elliott via Phabricator via cfe-commits]

lenary added a comment.

Note, D68862  is in-progress at the moment, 
which is related to this patch.


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] D67409: [RISCV] enable LTO support, pass some options to linker.

[Sam Elliott via Phabricator via cfe-commits]

lenary added a comment.

@khchen do you need me to commit this for you?


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

https://reviews.llvm.org/D67409



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

[PATCH] D67409: [RISCV] enable LTO support, pass some options to linker.

[Sam Elliott via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGcdcf58e5af02: [RISCV] enable LTO support, pass some options 
to linker. (authored by lenary).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67409

Files:
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/lib/Driver/ToolChains/CommonArgs.h
  clang/lib/Driver/ToolChains/RISCVToolchain.cpp
  clang/lib/Driver/ToolChains/RISCVToolchain.h
  clang/test/Driver/gold-lto.c

Index: clang/test/Driver/gold-lto.c
===
--- clang/test/Driver/gold-lto.c
+++ clang/test/Driver/gold-lto.c
@@ -26,3 +26,21 @@
 // RUN: %clang -target i686-linux-android -### %t.o -flto 2>&1 \
 // RUN: | FileCheck %s --check-prefix=CHECK-X86-ANDROID
 // CHECK-X86-ANDROID: "-plugin" "{{.*}}{{[/\\]}}LLVMgold.{{dll|dylib|so}}"
+//
+// RUN: %clang -target riscv64-unknown-elf -### %t.o -flto 2>&1 \
+// RUN: -march=rv64imf -mabi=lp64f \
+// RUN: | FileCheck %s --check-prefix=CHECK-RISCV-BAREMETAL
+// CHECK-RISCV-BAREMETAL: "-plugin" "{{.*}}{{[/\\]}}LLVMgold.{{dll|dylib|so}}"
+// CHECK-RISCV-BAREMETAL: "-plugin-opt=-mattr=+m"
+// CHECK-RISCV-BAREMETAL: "-plugin-opt=-mattr=+f"
+// CHECK-RISCV-BAREMETAL: "-plugin-opt=-mattr=+relax"
+// CHECK-RISCV-BAREMETAL: "-plugin-opt=-target-abi=lp64f"
+//
+// RUN: %clang -target riscv64-unknown-linux-gnu -### %t.o -flto 2>&1 \
+// RUN: -march=rv64imf -mabi=lp64f \
+// RUN: | FileCheck %s --check-prefix=CHECK-RISCV-LINUX
+// CHECK-RISCV-LINUX: "-plugin" "{{.*}}{{[/\\]}}LLVMgold.{{dll|dylib|so}}"
+// CHECK-RISCV-LINUX: "-plugin-opt=-mattr=+m"
+// CHECK-RISCV-LINUX: "-plugin-opt=-mattr=+f"
+// CHECK-RISCV-LINUX: "-plugin-opt=-mattr=+relax"
+// CHECK-RISCV-LINUX: "-plugin-opt=-target-abi=lp64f"
Index: clang/lib/Driver/ToolChains/RISCVToolchain.h
===
--- clang/lib/Driver/ToolChains/RISCVToolchain.h
+++ clang/lib/Driver/ToolChains/RISCVToolchain.h
@@ -25,6 +25,7 @@
   void addClangTargetOptions(const llvm::opt::ArgList &DriverArgs,
  llvm::opt::ArgStringList &CC1Args,
  Action::OffloadKind) const override;
+  bool HasNativeLLVMSupport() const override { return true; }
   void
   AddClangSystemIncludeArgs(const llvm::opt::ArgList &DriverArgs,
 llvm::opt::ArgStringList &CC1Args) const override;
Index: clang/lib/Driver/ToolChains/RISCVToolchain.cpp
===
--- clang/lib/Driver/ToolChains/RISCVToolchain.cpp
+++ clang/lib/Driver/ToolChains/RISCVToolchain.cpp
@@ -7,6 +7,7 @@
 //===--===//
 
 #include "RISCVToolchain.h"
+#include "Arch/RISCV.h"
 #include "CommonArgs.h"
 #include "InputInfo.h"
 #include "clang/Driver/Compilation.h"
@@ -100,6 +101,12 @@
 
   std::string Linker = getToolChain().GetProgramPath(getShortName());
 
+  if (D.isUsingLTO()) {
+assert(!Inputs.empty() && "Must have at least one input.");
+AddGoldPlugin(ToolChain, Args, CmdArgs, Output, Inputs[0],
+  D.getLTOMode() == LTOK_Thin);
+  }
+
   bool WantCRTs =
   !Args.hasArg(options::OPT_nostdlib, options::OPT_nostartfiles);
 
Index: clang/lib/Driver/ToolChains/CommonArgs.h
===
--- clang/lib/Driver/ToolChains/CommonArgs.h
+++ clang/lib/Driver/ToolChains/CommonArgs.h
@@ -118,6 +118,14 @@
 void addMultilibFlag(bool Enabled, const char *const Flag,
  Multilib::flags_list &Flags);
 
+StringRef getTargetABI(const llvm::opt::ArgList &Args,
+   const llvm::Triple &Triple);
+
+void getTargetFeatures(const ToolChain &TC, const llvm::Triple &Triple,
+   const llvm::opt::ArgList &Args,
+   llvm::opt::ArgStringList &CmdArgs, bool ForAS,
+   bool ForLTOPlugin = false);
+
 } // end namespace tools
 } // end namespace driver
 } // end namespace clang
Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -11,8 +11,12 @@
 #include "Arch/ARM.h"
 #include "Arch/Mips.h"
 #include "Arch/PPC.h"
+#include "Arch/RISCV.h"
+#include "Arch/Sparc.h"
 #include "Arch/SystemZ.h"
 #include "Arch/X86.h"
+#include "AMDGPU.h"
+#include "MSP430.h"
 #include "HIP.h"
 #include "Hexagon.h"
 #include "InputInfo.h"
@@ -484,6 +488,14 @@
   if (!StatsFile.empty())
 CmdArgs.push_back(
 Args.MakeArgString(Twine("-plugin-opt=stats-file=") + StatsFile));
+
+  getTargetFeatures(ToolChain, ToolChain.getTriple(), Args, CmdArgs,
+/* ForAS= */ false, /* ForLTOPlugin= 

[PATCH] D67409: [RISCV] enable LTO support, pass some options to linker.

[Sam Elliott via Phabricator via cfe-commits]

lenary added a comment.

@khchen Thanks for your patch! It is now landed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67409



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

[PATCH] D67508: [RISCV] support mutilib in baremetal environment

[Sam Elliott via Phabricator via cfe-commits]

lenary added a comment.

Please can you rebase these changes? Something has changed in 
RISCVToolchain.cpp and they are failing to build.

Q: Is there a plan to support multilib aliases (`MULTILIB_REUSE`)? I'm happy 
for this to be in a follow-up patch, would just like to know the plan.




Comment at: clang/lib/Driver/ToolChains/Gnu.cpp:1521
+  {"rv32i", "ilp32"},{"rv32im", "ilp32"}, {"rv32iac", "ilp32"},
+  {"rv32imac", "ilp32"}, {"rv32imafc", "ilp32f"}, {"rv64i", "lp64"},
+  {"rv64imac", "lp64"},  {"rv64imafdc", "lp64d"}};

I cannot see `march=rv64i/mabi=lp64` in [[ 
https://github.com/riscv/riscv-gcc/blob/ed3f6ec/gcc/config/riscv/t-elf-multilib 
| riscv-gcc t-elf-multilib ]] (the given sha is pulled from `.gitmodules` in 
riscv-gcc-toolchain. 


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

https://reviews.llvm.org/D67508



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

[PATCH] D68407: [RISCV] Use compiler-rt if no GCC installation detected

[Sam Elliott via Phabricator via cfe-commits]

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

Please can you add a test for riscv32 and riscv64 without libgcc?

I also think you want to be smarter about detecting the need/request for 
libgcc. Look for `AddRunTimeLibs` in clang/lib/Driver/ToolChains/CommonArgs.cpp 
- I think that will get you quite far. I think you can override 
`getCompilerRTArgString` for your toolchain, and also 
`GetDefaultRuntimeLibType`. For the correct compiler-rt stuff, you want 
`ToolChain::getCompilerRT` I think. Using all these methods should make these 
path additions a lot more robust.


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

https://reviews.llvm.org/D68407



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

[PATCH] D68362: [libunwind][RISCV] Add 64-bit RISC-V support

[Sam Elliott via Phabricator via cfe-commits]

lenary added a comment.

One query, but this patch is starting to look good.

I'm not a libunwind expert - it would be good to have one of the libunwind 
contributors look over this patch yet. Can you add one as a reviewer?




Comment at: libunwind/src/Registers.hpp:3677
+  case UNW_RISCV_X31:
+return "t6";
+  case UNW_RISCV_F0:

Good catch, thanks!



Comment at: libunwind/src/Registers.hpp:3756
+inline double Registers_riscv::getFloatRegister(int regNum) const {
+#ifdef __riscv_float_abi_double
+  assert(validFloatRegister(regNum));

Is this an ABI or an architecture issue? I'm not sure what other libunwind 
"backends" do for similar cases.

The difference is, if I compile libunwind with `-march=rv64g -mabi=lp64`, 
`__riscv_float_abi_double` is not defined (because you're using a soft-float 
ABI), but `__riscv_flen == 64` (because the machine does have hardware 
floating-point registers).

I'm not sure what the intended behaviour of libunwind is in this case.

`__riscv_float_abi_double` implies `__riscv_flen >= 64`.



Comment at: libunwind/src/UnwindCursor.hpp:999
+#if defined (_LIBUNWIND_TARGET_RISCV)
+  int stepWithCompactEncoding(Registers_riscv &) {
+return UNW_EINVAL;

mhorne wrote:
> lenary wrote:
> > Nit: This should be formatted like the version for `Registers_sparc` above.
> `Registers_sparc` is the outlier in this case, I've formatted it as all the 
> other architectures use. If you'd like I could fix the SPARC formatting in 
> this patch.
Ah, no worries then. Don't update the sparc formatting in this patch.


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

https://reviews.llvm.org/D68362



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

[PATCH] D68391: [RISCV] Improve sysroot computation if no GCC install detected

[Sam Elliott via Phabricator via cfe-commits]

lenary added a comment.

In D68391#1694622 , @edward-jones 
wrote:

> Rebased and added tests
>
> I've made this use the Triple from the driver rather than the parsed LLVM 
> triple, this means the Triple doesn't get normalized which seems like more 
> desirable behavior.


Yeah, using the user-provided triple seems the correct choice, as the user will 
expect it to match the directory name. Would be useful to add a comment saying 
something along the same lines where you do that.

> I've added to the riscv{32,64}-toolchain.c test files, however the added 
> tests cannot be run without a shell so I've had to disable those tests on 
> Windows. If necessary I can split these new tests out into separate files.

I think splitting the tests might be sensible, so that we can run as many as 
possible on platforms without a shell/windows.

> I realize that there don't appear to be any tests for the case where no GCC 
> install is found and no sysroot is provided to the driver. At the moment this 
> will result in a generic linker command using the system linker, such as:
> 
> /usr/bin/ld crt0.o crtbegin.o ... -lgcc crtend.o
> 
> Is this the desired behaviour? And if so should a test be added for this too?

I think defaulting to root if no sysroot/gcc install is found is better than 
erroring. In all likelihood a compile/link task will fail anyway because it 
cannot link the results together. In the case where the compile/link does work, 
then there's no issue. It would be useful to have a test for this.

In D68391#1707652 , @luismarques wrote:

> This is indeed an issue that would be nice to fix, I've often been annoyed by 
> clang just defaulting to the root when some misconfiguration occurs. I have 
> to wonder though, this patch only changes the clang RISC-V toolchain driver, 
> but the problem isn't specific to RISC-V. Couldn't this tweak be generalized 
> and made to apply to multiple/all target drivers?


I think that for baremetal risc-v, we can change this default without asking 
the wider community. For other targets, someone should email cfe-dev first with 
the proposal. I agree it makes sense, but I also imagine it could break a lot 
of builds unexpectedly.


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

https://reviews.llvm.org/D68391



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

[PATCH] D68362: [libunwind][RISCV] Add 64-bit RISC-V support

[Sam Elliott via Phabricator via cfe-commits]

lenary added inline comments.



Comment at: libunwind/src/Registers.hpp:3756
+inline double Registers_riscv::getFloatRegister(int regNum) const {
+#ifdef __riscv_float_abi_double
+  assert(validFloatRegister(regNum));

mhorne wrote:
> lenary wrote:
> > Is this an ABI or an architecture issue? I'm not sure what other libunwind 
> > "backends" do for similar cases.
> > 
> > The difference is, if I compile libunwind with `-march=rv64g -mabi=lp64`, 
> > `__riscv_float_abi_double` is not defined (because you're using a 
> > soft-float ABI), but `__riscv_flen == 64` (because the machine does have 
> > hardware floating-point registers).
> > 
> > I'm not sure what the intended behaviour of libunwind is in this case.
> > 
> > `__riscv_float_abi_double` implies `__riscv_flen >= 64`.
> An ABI issue, in my opinion. The unwind frame will always contain space for 
> the float registers, but accessing them should be disallowed for soft-float 
> configurations as the intent of soft-float is that the FPRs will not be used 
> at all. I'd say there is precedent for this in the MIPS implementation, since 
> it checks for `defined(__mips_hard_float) && __mips_fpr == 64`.
I had a discussion with @asb about this. The ISA vs ABI issue in RISC-V is 
complex. The TL;DR is we both think you need to be using `__riscv_flen == 64` 
here.

The reason for this is that if you compile with `-march=rv64imfd` but 
`-mabi=lp64`, the architecture still has floating point registers, it just does 
not use the floating-point calling convention. This means there are still 
`D`-extension instructions in the stream of instructions, just that "No 
floating-point registers, if present, are preserved across calls." (see [[ 
https://github.com/riscv/riscv-elf-psabi-doc/blob/master/riscv-elf.md#integer-calling-convention
 | psABI Integer Calling Convention ]]) This effectively means that with this 
combination, `f0-f31` are treated exactly the same as `t0-t6`, and so should be 
able to be restored when unwinding. It is not necessarily the case that with a 
soft float ABI, `f0-f31` are not used at all. This is similar to ARM's `soft` 
vs `softfp` calling conventions.

The expectation is that if you are compiling your programs with a specific 
`-march`, then you should be compiling your runtime libraries with the same 
`-march`. Eventually there should be enough details in the ELF file to allow 
you to ensure both `-march` and `-mabi` match when linking programs, but 
support for this is not widespread.


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

https://reviews.llvm.org/D68362



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

[PATCH] D67508: [RISCV] support mutilib in baremetal environment

[Sam Elliott via Phabricator via cfe-commits]

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

I finally got my system well setup enough to check this patch with my risc-v 
toolchains. It looks good, I'm happy for this to land.

There's no requirement to support MULTILIB_REUSE yet. It might be nice to add 
support in a follow-up patch, if you want.


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

https://reviews.llvm.org/D67508



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

[PATCH] D67508: [RISCV] support mutilib in baremetal environment

[Sam Elliott via Phabricator via cfe-commits]

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

Sorry for approving, and then requesting changes. I've been investigating 
issues with this patch.

When I try to use `-print-multi-lib` (a clang option that is very 
under-documented, but I think it's there to match GCC's option), this often 
doesn't always print out available multilibs, which is confusing.

1. `clang --gcc-toolchain=<..> --target=riscv32-unknown-elf -print-multi-lib` 
does print out available multilibs (as gcc would)
2. `clang --gcc-toolchain=<..> --target=riscv64-unknown-elf -print-multi-lib` 
does not print out anything.
3. `clang --gcc-toolchain=<..> --target=riscv64-unknown-elf -march=rv64gc 
-mabi=lp64d -print-multi-lib` does not print anything.
4. `clang --gcc-toolchain=<..> --target=riscv64-unknown-elf -march=rv64imafdc 
-mabi=lp64d -print-multi-lib` does print out available multilibs (as gcc would).

In all cases, I'm using a crosstool-ng configured toolchain for 
`riscv64-unknown-elf`.

I get the correct output for `2.` if I change the line noted in the line 
comment below. Note that choosing a reasonable default here has to match other 
defaults chosen in clang.

I think the reason that `3.` is not working is because of MULTILIB_REUSE. I do 
not think that blocks this patch, as I've said, this patch does not need to 
include MULTILIB_REUSE support.




Comment at: clang/lib/Driver/ToolChains/Gnu.cpp:1547
+  else if (IsRV64)
+MArch = "rv64i";
+  else

I think this line is the issue: where someone doesn't specify `-march`, you 
choose a default `-march` that does not appear in the list of multilib arches 
in `RISCVMultilibSet` above.

I think this issue probably didn't arise when you had `rv64i/lp64` in the list, 
but given that's not in the riscv-gcc `t-elf-multilib`, so this code should 
choose a new default. I don't know how this new default will affect defaults 
chosen in other parts of the clang codebase.


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

https://reviews.llvm.org/D67508



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

[PATCH] D67508: [RISCV] support mutilib in baremetal environment

[Sam Elliott via Phabricator via cfe-commits]

lenary added a comment.

Ok, found a path forward for this patch. Notes inline:




Comment at: clang/lib/Driver/ToolChains/Gnu.cpp:1547
+  else if (IsRV64)
+MArch = "rv64i";
+  else

khchen wrote:
> lenary wrote:
> > I think this line is the issue: where someone doesn't specify `-march`, you 
> > choose a default `-march` that does not appear in the list of multilib 
> > arches in `RISCVMultilibSet` above.
> > 
> > I think this issue probably didn't arise when you had `rv64i/lp64` in the 
> > list, but given that's not in the riscv-gcc `t-elf-multilib`, so this code 
> > should choose a new default. I don't know how this new default will affect 
> > defaults chosen in other parts of the clang codebase.
> oh it's why I added rv64i/lp64 in previous version, but it's wrong default.
> in riscv gcc if configure only with --enable-multilib, the default march/abi 
> is rv64imafdc/lp64d
> The option 1 is aligning to gcc, but the default abi is lp64 in clang,
> maybe chosing rv64imac as default is less side effect? what do you think?
The solution for this patch is to use the function `riscv::getRISCVABI(const 
ArgList &Args, const llvm::Triple &Triple)` from 
`clang/lib/Driver/ToolChains/Arch/RISCV.h`. 

Then when I update the logic there to match gcc's logic, the right thing will 
happen. I will submit a patch to have this function match the gcc logic today. 
It took me a few hours but I found the default logic for GCC. 


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

https://reviews.llvm.org/D67508



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

[PATCH] D67508: [RISCV] support mutilib in baremetal environment

[Sam Elliott via Phabricator via cfe-commits]

lenary added inline comments.



Comment at: clang/lib/Driver/ToolChains/Gnu.cpp:1547
+  else if (IsRV64)
+MArch = "rv64i";
+  else

lenary wrote:
> khchen wrote:
> > lenary wrote:
> > > I think this line is the issue: where someone doesn't specify `-march`, 
> > > you choose a default `-march` that does not appear in the list of 
> > > multilib arches in `RISCVMultilibSet` above.
> > > 
> > > I think this issue probably didn't arise when you had `rv64i/lp64` in the 
> > > list, but given that's not in the riscv-gcc `t-elf-multilib`, so this 
> > > code should choose a new default. I don't know how this new default will 
> > > affect defaults chosen in other parts of the clang codebase.
> > oh it's why I added rv64i/lp64 in previous version, but it's wrong default.
> > in riscv gcc if configure only with --enable-multilib, the default 
> > march/abi is rv64imafdc/lp64d
> > The option 1 is aligning to gcc, but the default abi is lp64 in clang,
> > maybe chosing rv64imac as default is less side effect? what do you think?
> The solution for this patch is to use the function `riscv::getRISCVABI(const 
> ArgList &Args, const llvm::Triple &Triple)` from 
> `clang/lib/Driver/ToolChains/Arch/RISCV.h`. 
> 
> Then when I update the logic there to match gcc's logic, the right thing will 
> happen. I will submit a patch to have this function match the gcc logic 
> today. It took me a few hours but I found the default logic for GCC. 
Sorry, I'm wrong. I'm adding a `riscv::getRISCVArch(...)` to that file, which 
will solve this issue, sorry for the confusion. Will add this patch as 
dependent on that one when it's ready.


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

https://reviews.llvm.org/D67508



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

[PATCH] D69383: [RISCV] Match GCC `-march`/`-mabi` driver defaults

[Sam Elliott via Phabricator via cfe-commits]

lenary created this revision.
lenary added reviewers: asb, luismarques, rogfer01, kito-cheng, khchen.
Herald added subscribers: cfe-commits, pzheng, s.egerton, Jim, benna, psnobl, 
jocewei, PkmX, rkruppe, the_o, brucehoult, MartinMosbeck, edward-jones, zzheng, 
MaskRay, jrtc27, shiva0217, niosHD, sabuasal, apazos, simoncook, johnrusso, 
rbar.
Herald added a project: clang.
lenary added a child revision: D67508: [RISCV] support mutilib in baremetal 
environment.
lenary updated this revision to Diff 226252.
lenary added a comment.

- Correct code formatting issue


GCC is not a cross-compiler, and has convoluted logic in its build
system to choose a default `-march`/`-mabi` based on build options.

Clang/LLVM is a cross-compiler, and so we don't have to make a choice
about `-march`/`-mabi` at build-time, but we may have to compute a
default `-march`/`-mabi` when compiling a program.

This patch adds a new function `riscv::getRISCVArch` which encapsulates
the logic in GCC's build system for computing a default `-march` value
when none is provided. This patch also updates the logic in
`riscv::getRISCVABI` to match the logic in GCC's build system for
computing a default `-mabi`.

This patch also updates anywhere that `-march` is used to now use the
new function which can compute a default. In particular, we now
explicitly pass a `-march` value down to the gnu assembler.

Tests have been updated to match the new logic.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D69383

Files:
  clang/lib/Driver/ToolChains/Arch/RISCV.cpp
  clang/lib/Driver/ToolChains/Arch/RISCV.h
  clang/lib/Driver/ToolChains/Gnu.cpp
  clang/test/Driver/riscv-abi.c
  clang/test/Driver/riscv-gnutools.c

Index: clang/test/Driver/riscv-gnutools.c
===
--- clang/test/Driver/riscv-gnutools.c
+++ clang/test/Driver/riscv-gnutools.c
@@ -1,19 +1,19 @@
 // Check gnutools are invoked with propagated values for -mabi and -march.
 
 // RUN: %clang -target riscv32 -fno-integrated-as %s -###  -c \
-// RUN: 2>&1 | FileCheck -check-prefix=MABI-ILP32 %s
+// RUN: 2>&1 | FileCheck -check-prefix=MABI-ILP32D %s
 
 // RUN: %clang -target riscv32 -fno-integrated-as -march=rv32g %s -### -c \
-// RUN: 2>&1 | FileCheck -check-prefix=MABI-ILP32-MARCH-G %s
+// RUN: 2>&1 | FileCheck -check-prefix=MABI-ILP32D-MARCH-G %s
 
 // RUN: %clang -target riscv64 -fno-integrated-as %s -###  -c \
-// RUN: 2>&1 | FileCheck -check-prefix=MABI-ILP64 %s
+// RUN: 2>&1 | FileCheck -check-prefix=MABI-ILP64D %s
 
 // RUN: %clang -target riscv64 -fno-integrated-as -march=rv64g %s -### -c \
-// RUN: 2>&1 | FileCheck -check-prefix=MABI-ILP64-MARCH-G %s
+// RUN: 2>&1 | FileCheck -check-prefix=MABI-ILP64D-MARCH-G %s
 
-// MABI-ILP32: "{{.*}}as{{(.exe)?}}" "-mabi" "ilp32"
-// MABI-ILP32-MARCH-G: "{{.*}}as{{(.exe)?}}" "-mabi" "ilp32" "-march" "rv32g"
+// MABI-ILP32D: "{{.*}}as{{(.exe)?}}" "-mabi" "ilp32d" "-march" "rv32gc"
+// MABI-ILP32D-MARCH-G: "{{.*}}as{{(.exe)?}}" "-mabi" "ilp32d" "-march" "rv32g"
 
-// MABI-ILP64: "{{.*}}as{{(.exe)?}}" "-mabi" "lp64"
-// MABI-ILP64-MARCH-G: "{{.*}}as{{(.exe)?}}" "-mabi" "lp64" "-march" "rv64g"
+// MABI-ILP64D: "{{.*}}as{{(.exe)?}}" "-mabi" "lp64d"
+// MABI-ILP64D-MARCH-G: "{{.*}}as{{(.exe)?}}" "-mabi" "lp64d" "-march" "rv64g"
Index: clang/test/Driver/riscv-abi.c
===
--- clang/test/Driver/riscv-abi.c
+++ clang/test/Driver/riscv-abi.c
@@ -1,9 +1,5 @@
-// RUN: %clang -target riscv32-unknown-elf %s -### -o %t.o 2>&1 \
-// RUN:   | FileCheck -check-prefix=CHECK-ILP32 %s
 // RUN: %clang -target riscv32-unknown-elf %s -### -o %t.o -mabi=ilp32 2>&1 \
 // RUN:   | FileCheck -check-prefix=CHECK-ILP32 %s
-// RUN: %clang -target riscv32-unknown-elf -x assembler %s -### -o %t.o 2>&1 \
-// RUN:   | FileCheck -check-prefix=CHECK-ILP32 %s
 // RUN: %clang -target riscv32-unknown-elf -x assembler %s -### -o %t.o \
 // RUN:   -mabi=ilp32 2>&1 | FileCheck -check-prefix=CHECK-ILP32 %s
 
@@ -14,6 +10,10 @@
 
 // CHECK-ILP32F: "-target-abi" "ilp32f"
 
+// RUN: %clang -target riscv32-unknown-elf %s -### -o %t.o 2>&1 \
+// RUN:   | FileCheck -check-prefix=CHECK-ILP32D %s
+// RUN: %clang -target riscv32-unknown-elf -x assembler %s -### -o %t.o 2>&1 \
+// RUN:   | FileCheck -check-prefix=CHECK-ILP32D %s
 // RUN: %clang -target riscv32-unknown-elf %s -### -o %t.o -march=rv32ifd -mabi=ilp32d 2>&1 \
 // RUN:   | FileCheck -check-prefix=CHECK-ILP32D %s
 
@@ -24,12 +24,8 @@
 
 // CHECK-RV32-LP64: error: unknown target ABI 'lp64'
 
-// RUN: %clang -target riscv64-unknown-elf %s -### -o %t.o 2>&1 \
-// RUN:   | FileCheck -check-prefix=CHECK-LP64 %s
 // RUN: %clang -target riscv64-unknown-elf %s -### -o %t.o -mabi=lp64 2>&1 \
 // RUN:   | FileCheck -check-prefix=CHECK-LP64 %s
-// RUN: %clang -target riscv64-unknown-elf -x assembler %s -### -o %t.o 2>&1 \
-// RUN:   | FileCheck -check-prefix=CHECK-LP64  %s
 // RUN: %clang -target riscv64-unknown-elf -

[PATCH] D69383: [RISCV] Match GCC `-march`/`-mabi` driver defaults

[Sam Elliott via Phabricator via cfe-commits]

lenary updated this revision to Diff 226252.
lenary added a comment.

- Correct code formatting issue


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69383

Files:
  clang/lib/Driver/ToolChains/Arch/RISCV.cpp
  clang/lib/Driver/ToolChains/Arch/RISCV.h
  clang/lib/Driver/ToolChains/Gnu.cpp
  clang/test/Driver/riscv-abi.c
  clang/test/Driver/riscv-gnutools.c

Index: clang/test/Driver/riscv-gnutools.c
===
--- clang/test/Driver/riscv-gnutools.c
+++ clang/test/Driver/riscv-gnutools.c
@@ -1,19 +1,19 @@
 // Check gnutools are invoked with propagated values for -mabi and -march.
 
 // RUN: %clang -target riscv32 -fno-integrated-as %s -###  -c \
-// RUN: 2>&1 | FileCheck -check-prefix=MABI-ILP32 %s
+// RUN: 2>&1 | FileCheck -check-prefix=MABI-ILP32D %s
 
 // RUN: %clang -target riscv32 -fno-integrated-as -march=rv32g %s -### -c \
-// RUN: 2>&1 | FileCheck -check-prefix=MABI-ILP32-MARCH-G %s
+// RUN: 2>&1 | FileCheck -check-prefix=MABI-ILP32D-MARCH-G %s
 
 // RUN: %clang -target riscv64 -fno-integrated-as %s -###  -c \
-// RUN: 2>&1 | FileCheck -check-prefix=MABI-ILP64 %s
+// RUN: 2>&1 | FileCheck -check-prefix=MABI-ILP64D %s
 
 // RUN: %clang -target riscv64 -fno-integrated-as -march=rv64g %s -### -c \
-// RUN: 2>&1 | FileCheck -check-prefix=MABI-ILP64-MARCH-G %s
+// RUN: 2>&1 | FileCheck -check-prefix=MABI-ILP64D-MARCH-G %s
 
-// MABI-ILP32: "{{.*}}as{{(.exe)?}}" "-mabi" "ilp32"
-// MABI-ILP32-MARCH-G: "{{.*}}as{{(.exe)?}}" "-mabi" "ilp32" "-march" "rv32g"
+// MABI-ILP32D: "{{.*}}as{{(.exe)?}}" "-mabi" "ilp32d" "-march" "rv32gc"
+// MABI-ILP32D-MARCH-G: "{{.*}}as{{(.exe)?}}" "-mabi" "ilp32d" "-march" "rv32g"
 
-// MABI-ILP64: "{{.*}}as{{(.exe)?}}" "-mabi" "lp64"
-// MABI-ILP64-MARCH-G: "{{.*}}as{{(.exe)?}}" "-mabi" "lp64" "-march" "rv64g"
+// MABI-ILP64D: "{{.*}}as{{(.exe)?}}" "-mabi" "lp64d"
+// MABI-ILP64D-MARCH-G: "{{.*}}as{{(.exe)?}}" "-mabi" "lp64d" "-march" "rv64g"
Index: clang/test/Driver/riscv-abi.c
===
--- clang/test/Driver/riscv-abi.c
+++ clang/test/Driver/riscv-abi.c
@@ -1,9 +1,5 @@
-// RUN: %clang -target riscv32-unknown-elf %s -### -o %t.o 2>&1 \
-// RUN:   | FileCheck -check-prefix=CHECK-ILP32 %s
 // RUN: %clang -target riscv32-unknown-elf %s -### -o %t.o -mabi=ilp32 2>&1 \
 // RUN:   | FileCheck -check-prefix=CHECK-ILP32 %s
-// RUN: %clang -target riscv32-unknown-elf -x assembler %s -### -o %t.o 2>&1 \
-// RUN:   | FileCheck -check-prefix=CHECK-ILP32 %s
 // RUN: %clang -target riscv32-unknown-elf -x assembler %s -### -o %t.o \
 // RUN:   -mabi=ilp32 2>&1 | FileCheck -check-prefix=CHECK-ILP32 %s
 
@@ -14,6 +10,10 @@
 
 // CHECK-ILP32F: "-target-abi" "ilp32f"
 
+// RUN: %clang -target riscv32-unknown-elf %s -### -o %t.o 2>&1 \
+// RUN:   | FileCheck -check-prefix=CHECK-ILP32D %s
+// RUN: %clang -target riscv32-unknown-elf -x assembler %s -### -o %t.o 2>&1 \
+// RUN:   | FileCheck -check-prefix=CHECK-ILP32D %s
 // RUN: %clang -target riscv32-unknown-elf %s -### -o %t.o -march=rv32ifd -mabi=ilp32d 2>&1 \
 // RUN:   | FileCheck -check-prefix=CHECK-ILP32D %s
 
@@ -24,12 +24,8 @@
 
 // CHECK-RV32-LP64: error: unknown target ABI 'lp64'
 
-// RUN: %clang -target riscv64-unknown-elf %s -### -o %t.o 2>&1 \
-// RUN:   | FileCheck -check-prefix=CHECK-LP64 %s
 // RUN: %clang -target riscv64-unknown-elf %s -### -o %t.o -mabi=lp64 2>&1 \
 // RUN:   | FileCheck -check-prefix=CHECK-LP64 %s
-// RUN: %clang -target riscv64-unknown-elf -x assembler %s -### -o %t.o 2>&1 \
-// RUN:   | FileCheck -check-prefix=CHECK-LP64  %s
 // RUN: %clang -target riscv64-unknown-elf -x assembler %s -### -o %t.o \
 // RUN:   -mabi=lp64 2>&1 | FileCheck -check-prefix=CHECK-LP64 %s
 
@@ -40,6 +36,10 @@
 
 // CHECK-LP64F: "-target-abi" "lp64f"
 
+// RUN: %clang -target riscv64-unknown-elf %s -### -o %t.o 2>&1 \
+// RUN:   | FileCheck -check-prefix=CHECK-LP64D %s
+// RUN: %clang -target riscv64-unknown-elf -x assembler %s -### -o %t.o 2>&1 \
+// RUN:   | FileCheck -check-prefix=CHECK-LP64D  %s
 // RUN: %clang -target riscv64-unknown-elf %s -### -o %t.o -march=rv64d -mabi=lp64d 2>&1 \
 // RUN:   | FileCheck -check-prefix=CHECK-LP64D %s
 
Index: clang/lib/Driver/ToolChains/Gnu.cpp
===
--- clang/lib/Driver/ToolChains/Gnu.cpp
+++ clang/lib/Driver/ToolChains/Gnu.cpp
@@ -709,11 +709,9 @@
 StringRef ABIName = riscv::getRISCVABI(Args, getToolChain().getTriple());
 CmdArgs.push_back("-mabi");
 CmdArgs.push_back(ABIName.data());
-if (const Arg *A = Args.getLastArg(options::OPT_march_EQ)) {
-  StringRef MArch = A->getValue();
-  CmdArgs.push_back("-march");
-  CmdArgs.push_back(MArch.data());
-}
+StringRef MArchName = riscv::getRISCVArch(Args, getToolChain().getTriple());
+CmdArgs.push_back("-march");
+CmdArgs.push_back(MArchName.data());

[PATCH] D69383: [RISCV] Match GCC `-march`/`-mabi` driver defaults

[Sam Elliott via Phabricator via cfe-commits]

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



Comment at: clang/lib/Driver/ToolChains/Arch/RISCV.cpp:475
+
+if (MArch.startswith_lower("rv32")) {
+  if (MArch.substr(4).contains_lower("d") ||

rogfer01 wrote:
> `llvm::StringSwitch` has a method `StartsWithLower` which might help make the 
> logic a bit clearer
> 
> Something like this (I haven't tested it!)
> 
> ```lang=cpp
> StringRef ABIFromMarch = StringSwitch(MArch)
>.StartsWithLower("rv32d", "ilp32d")
>.StartsWithLower("rv32g", "ilp32d")
>.StartsWithLower("rv32e", "ilp32e")
>.StartsWithLower("rv32", "ilp32")
> 
>.StartsWithLower("rv64d", "lp64d")
>.StartsWithLower("rv64g", "lp64d")
>.StartsWithLower("rv64", "lp64").
> 
>.Default("");
> 
> if (!ABIFromMarch.empty()) return ABIFromMarch;
> ```
Sadly I don't think this will work, because of the case of matching `rv32*d*` 
and `rv64*d*` (the `March.substr(4).contains_lower("d")` cases) from 
config.gcc. Explicitly "d" does not come immediately after `rv<32/64>`, it can 
come anywhere after like in `rv32imafdc`.

The other issue I have with the StringSwitch is that it requires I have a 
default, which I feel makes the control flow harder to understand, rather than 
easier. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69383



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

[PATCH] D69383: [RISCV] Match GCC `-march`/`-mabi` driver defaults

[Sam Elliott via Phabricator via cfe-commits]

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



Comment at: clang/lib/Driver/ToolChains/Arch/RISCV.cpp:537
+  if (Triple.getArch() == llvm::Triple::riscv32)
+return "rv32gc";
   else

khchen wrote:
> Why do you set rv32gc and rv64gc as default march? Is there a any reason?
This reflects the logic in [[ 
https://github.com/riscv/riscv-gcc/blob/riscv-gcc-9.2.0/gcc/config.gcc#L4229-L4369
 | `config.gcc` ]]. Note line 4273, which applies if neither arch nor abi is 
given (the xlen is set on line 4233-4234 based on the target triple).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69383



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

[PATCH] D67508: [RISCV] support mutilib in baremetal environment

[Sam Elliott via Phabricator via cfe-commits]

lenary added a comment.



In D67508#1720228 , @khchen wrote:

> But there is some issue if we set the default rv32 march as `rv32gc`. 
>  Because the default multilib does not include `rv32gc`/`lp32d` in riscv gnu 
> toolchain, 
> https://github.com/riscv/riscv-gcc/blob/ed3f6ec/gcc/config/riscv/t-elf-multilib#L19
>  so if user does not give the default march clang will not find the library 
> and cause linking error.
>  Do you think this is a clang's problem? or maybe this is just user's problem 
> so they should config multilib to support `rv32gc/lip3d2d`?


Oh, I'm seeing how these pieces (don't) fit together now. Sorry for missing 
that the defaults in D69383  don't quite line 
up with this patch.

~~What happens if a user requests a `t-multilib-elf` build of GCC, without 
specifying `--with-arch=` or `--with-abi=`, then they get a default march/mabi 
combination that they don't have a multilib for, or do the definitions come 
from somewhere else?~~ So I realise that the `--with-arch=` and `--with-abi=` 
options come from `t-elf-multilib` when gcc is built.

Looking closer at the `t-elf-multilib` file, there's not any `ilp32d` 
configuration, which seems like an oversight if people want to use that ABI on 
elf targets.

So, I guess there are three choices:

1. We change the defaults in `D69383` to match the elf multilibs that are built 
today. (i.e., one of the current multilib choices will become a default).
2. We request a change in the `t-elf-multilib` `MULTILIB_REQUIRED` list to add 
`rv32imafdc`/`ilp32d` (and add an `rv32gc`/`ilp32d` alias to `MULTILIB_REUSE`) 
(This probably requires rebuilding existing multilib configurations, to compile 
this extra combination)
3. We request a change in `config.gcc` (and I update my patch to reflect it) 
(This probably means having per-target defaults for linux vs elf, and ensuring 
these defaults are in the multilib file).

Part of the issue is it's not great to change defaults (we can probably only do 
it for clang 10, and never again), and it's not great to make people recompile 
their gcc toolchains either.


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

https://reviews.llvm.org/D67508



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

[PATCH] D69383: [RISCV] Match GCC `-march`/`-mabi` driver defaults

[Sam Elliott via Phabricator via cfe-commits]

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

I agree backwards compatibility is hard here.

- This method was introduced to have a single place to choose a default `march` 
string if none was chosen before. I think this change is useful (saves defaults 
being calculated in a multitude of other places, which means they may not 
agree).
- The choice of a default march string was based, as closely as possible, on 
`config.gcc`. This may not be what we want (and is the source of backwards 
compatibility issues).
- I 100% agree that a release notes entry is needed. I will write one just as 
soon as we finalize the default behaviour.
- The elf multilib changes in D67508  seem to 
be more complex, as `config.gcc` chooses a march/mabi default that is not built 
by `t-elf-multilib`.

I think I would understand updating the logic to use fewer architecture 
extensions for a given ABI (i.e., default to `rv32i` unless someone asks for 
`ilp32f/d`) rather than more (`rv32gc` when someone asks for `ilp32`). This 
should be more backwards compatible, but also requires carefully ensuring the 
logic is not circular.

At the very least, I do not plan to merge this patch until we have had a chance 
to discuss it next Thursday (31st Oct) on the RISC-V backend call.




Comment at: clang/lib/Driver/ToolChains/Arch/RISCV.cpp:476
+if (MArch.startswith_lower("rv32")) {
+  if (MArch.substr(4).contains_lower("d") ||
+  MArch.startswith_lower("rv32g"))

simoncook wrote:
> Won’t this break if the user specifies a X/Z extension that has a d in the 
> name, so eg rv32iXd will try to use the ilp32d abi by default? For future 
> proofing, I think we may need to do a full parse of the isa string to verify 
> that d does actually mean the standard D-extension
You're right that having "d" in any extension name will cause issues. The same 
is true for any gcc build today (9.2.0).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69383



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

[PATCH] D68391: [RISCV] Improve sysroot computation if no GCC install detected

[Sam Elliott via Phabricator via cfe-commits]

lenary accepted this revision.
lenary added a comment.
This revision is now accepted and ready to land.
Herald added a subscriber: sameer.abuasal.

LGTM.


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

https://reviews.llvm.org/D68391



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

[PATCH] D80963: [WIP][clang] Allow {u}int_fastN_t to be different to {u}int_leastN_t

[Sam Elliott via Phabricator via cfe-commits]

lenary created this revision.
lenary added reviewers: luismarques, asb.
Herald added a project: clang.

This is in order to support psABIs where these two type sizes do not match for
specific values of N. The default implementation matches clang's current
behaviour where `getLeastIntTypeByWidth` is used for `{u}int_fastN_t`.

This patch is a Work-In-Progress. I am seeking guidance as to how to update
clang's `stdint.h` to match this change.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D80963

Files:
  clang/include/clang/Basic/TargetInfo.h
  clang/lib/Frontend/InitPreprocessor.cpp


Index: clang/lib/Frontend/InitPreprocessor.cpp
===
--- clang/lib/Frontend/InitPreprocessor.cpp
+++ clang/lib/Frontend/InitPreprocessor.cpp
@@ -257,7 +257,7 @@
   const TargetInfo &TI, MacroBuilder &Builder) {
   // stdint.h currently defines the fast int types as equivalent to the least
   // types.
-  TargetInfo::IntType Ty = TI.getLeastIntTypeByWidth(TypeWidth, IsSigned);
+  TargetInfo::IntType Ty = TI.getFastIntTypeByWidth(TypeWidth, IsSigned);
   if (Ty == TargetInfo::NoInt)
 return;
 
Index: clang/include/clang/Basic/TargetInfo.h
===
--- clang/include/clang/Basic/TargetInfo.h
+++ clang/include/clang/Basic/TargetInfo.h
@@ -365,9 +365,21 @@
   virtual IntType getIntTypeByWidth(unsigned BitWidth, bool IsSigned) const;
 
   /// Return the smallest integer type with at least the specified width.
+  ///
+  /// This will be used for `{u}int_least_t` in C.
   virtual IntType getLeastIntTypeByWidth(unsigned BitWidth,
  bool IsSigned) const;
 
+  /// Return the "fastest" integer type with at least the specified width.
+  ///
+  /// This will be used for `{u}int_fast_t` in C.
+  ///
+  /// The default implementation will match getLeastIntTypeByWidth.
+  virtual IntType getFastIntTypeByWidth(unsigned BitWidth,
+bool IsSigned) const {
+return getLeastIntTypeByWidth(BitWidth, IsSigned);
+  }
+
   /// Return floating point type with specified width. On PPC, there are
   /// three possible types for 128-bit floating point: "PPC double-double",
   /// IEEE 754R quad precision, and "long double" (which under the covers


Index: clang/lib/Frontend/InitPreprocessor.cpp
===
--- clang/lib/Frontend/InitPreprocessor.cpp
+++ clang/lib/Frontend/InitPreprocessor.cpp
@@ -257,7 +257,7 @@
   const TargetInfo &TI, MacroBuilder &Builder) {
   // stdint.h currently defines the fast int types as equivalent to the least
   // types.
-  TargetInfo::IntType Ty = TI.getLeastIntTypeByWidth(TypeWidth, IsSigned);
+  TargetInfo::IntType Ty = TI.getFastIntTypeByWidth(TypeWidth, IsSigned);
   if (Ty == TargetInfo::NoInt)
 return;
 
Index: clang/include/clang/Basic/TargetInfo.h
===
--- clang/include/clang/Basic/TargetInfo.h
+++ clang/include/clang/Basic/TargetInfo.h
@@ -365,9 +365,21 @@
   virtual IntType getIntTypeByWidth(unsigned BitWidth, bool IsSigned) const;
 
   /// Return the smallest integer type with at least the specified width.
+  ///
+  /// This will be used for `{u}int_least_t` in C.
   virtual IntType getLeastIntTypeByWidth(unsigned BitWidth,
  bool IsSigned) const;
 
+  /// Return the "fastest" integer type with at least the specified width.
+  ///
+  /// This will be used for `{u}int_fast_t` in C.
+  ///
+  /// The default implementation will match getLeastIntTypeByWidth.
+  virtual IntType getFastIntTypeByWidth(unsigned BitWidth,
+bool IsSigned) const {
+return getLeastIntTypeByWidth(BitWidth, IsSigned);
+  }
+
   /// Return floating point type with specified width. On PPC, there are
   /// three possible types for 128-bit floating point: "PPC double-double",
   /// IEEE 754R quad precision, and "long double" (which under the covers
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D81083: [Clang] Allow "vector_size" applied to Booleans

[Sam Elliott via Phabricator via cfe-commits]

lenary added inline comments.



Comment at: clang/docs/LanguageExtensions.rst:473
+architectures.  The size parameter of a boolean vector type is the number of
+bits in the vector (for all non-bool vectors, the number refers to the number
+of bytes in the vector).

It would be nice if this aside about non-bool vectors was more prominently 
displayed - it's something I hadn't realised before.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81083



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

[PATCH] D71124: [RISCV] support clang driver to select cpu

[Sam Elliott via Phabricator via cfe-commits]

lenary added a comment.

I realise this is almost certainly something we want to land before the LLVM 11 
branch date, as we included schedules in LLVM 10 with no way to use them, and 
would like users to be able to use them. I'll bring it up on the call tomorrow 
- I hope this PR implements what we agreed from the previous calls.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71124



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

[PATCH] D71124: [RISCV] support clang driver to select cpu

[Sam Elliott via Phabricator via cfe-commits]

lenary added a comment.

I've got one major issue (inline below), and I'm confused by some other 
behaviour:

When I run `clang --target=riscv64 -mcpu=?`, the list includes both 
`generic-rv32` and `generic-rv64`. It doesn't show only the 64-bit cpus. This 
is not changed by giving a full triple, or by additionally specifying 
`-march=rv64gc`. Should it be?

For instance, this output:

  $ clang --target=riscv64-unknown-elf -mcpu=\? -march=rv64gc
  clang version 11.0.0
  Target: riscv64-unknown-unknown-elf
  Thread model: posix
  InstalledDir: /home/selliott/llvm-project/./build/llvm_clang/all/bin
  Available CPUs for this target:
  
generic-rv32
generic-rv64
rocket-rv32
rocket-rv64
sifive-e31
sifive-u54
  
  Use -mcpu or -mtune to specify the target's processor.
  For example, clang --target=aarch64-unknown-linux-gui -mcpu=cortex-a35




Comment at: clang/lib/Driver/ToolChains/Arch/RISCV.cpp:599
+  return "lp64";
   }
 }

I now get a stacktrace here if I call `clang --target=riscv32 -march=unknown 
foo.c -c`. In clang-10, you get a nice error:
```
clang-10: error: invalid arch name 'wat', string must begin with rv32{i,e,g} or 
rv64{i,g}
```

The stacktrace is coming from the call to `getRISCVABI` in 
`findRISCVBareMetalMultilibs` (in `clang/lib/Driver/ToolChains/Gnu.cpp:1608`).




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71124



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

[PATCH] D71124: [RISCV] support clang driver to select cpu

[Sam Elliott via Phabricator via cfe-commits]

lenary added a comment.

Thanks for the fix!

Please can you also update `clang/test/Misc/target-invalid-cpu-note.c` for 
riscv32 and riscv64 - it's likely you'll need the fix in the inline note below.

Also, there are a bunch of clang-tidy issues, which I think we can fix now 
rather than later.




Comment at: llvm/lib/Support/TargetParser.cpp:250
+  for (const auto &C : RISCVCPUInfo) {
+if (IsRV64 == C.Is64Bit())
+  Values.emplace_back(C.Name);

This line should be `if (C.Kind != CK_INVALID && IsRV64 == C.Is64Bit())` so 
that when clang prints out valid CPUs, 'invalid' is not included in the list. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71124



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

[PATCH] D71124: [RISCV] support clang driver to select cpu

[Sam Elliott via Phabricator via cfe-commits]

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

I am happy with this. I think we should get it landed so we can backport it to 
the LLVM 11 branch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71124



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

[PATCH] D71124: [RISCV] support clang driver to select cpu

[Sam Elliott via Phabricator via cfe-commits]

lenary added a comment.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

This is looking good.

I remember we discussed this on the LLVM call a few weeks ago - there was a 
discussion as to whether we should be prioritising `-march` or `-mcpu` - do you 
recall the outcome of that discussion?




Comment at: clang/lib/Basic/Targets/RISCV.cpp:166
+bool RISCV32TargetInfo::isValidCPUName(StringRef Name) const {
+  return llvm::RISCV::checkCPUKind(llvm::RISCV::parseCPUKind(Name), false);
+}

It would be good to have `/*Is64Bit=*/` before the boolean arguments to these 
functions.



Comment at: clang/lib/Driver/ToolChains/Arch/RISCV.cpp:501-516
-  // 3. Choose a default based on the triple
-  //
-  // We deviate from GCC's defaults here:
-  // - On `riscv{XLEN}-unknown-elf` we use the integer calling convention only.
-  // - On all other OSs we use the double floating point calling convention.
-  if (Triple.getArch() == llvm::Triple::riscv32) {
-if (Triple.getOS() == llvm::Triple::UnknownOS)

What's the justification for removing this code?



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

Should we be doing more validation here?



Comment at: llvm/lib/Support/TargetParser.cpp:122
 
+struct CPUInfo {
+  StringLiteral Name;

I think this should be prefixed `RISCV`, or moved into the RISCV namespace if 
you can?



Comment at: llvm/lib/Support/TargetParser.cpp:259-260
+
+  if (features & FK_64BIT)
+Features.push_back("+64bit");
+

It might be worth explicitly adding `-64bit` where FK_64BIT is not set.


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

https://reviews.llvm.org/D71124



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

[PATCH] D79322: [FEnv] Small fixes to implementation of flt.rounds

[Sam Elliott via Phabricator via cfe-commits]

lenary added inline comments.



Comment at: llvm/test/CodeGen/RISCV/flt-rounds.ll:23
+; RV64I-NEXT:sd ra, 8(sp)
+; RV64I-NEXT:call __flt_rounds
+; RV64I-NEXT:ld ra, 8(sp)

I'm interested to understand how this function is provided. Is it part of 
`compiler-rt` or `libgcc`, or is it provided another way?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79322



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

[PATCH] D128415: [ARM] Add Support for Cortex-M85

[Sam Elliott via Phabricator via cfe-commits]

lenary updated this revision to Diff 442079.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128415

Files:
  clang/docs/ReleaseNotes.rst
  clang/test/CodeGen/arm-target-features.c
  clang/test/Driver/arm-cortex-cpus-2.c
  clang/test/Driver/arm-nofp-disabled-features.c
  clang/test/Misc/target-invalid-cpu-note.c
  llvm/docs/ReleaseNotes.rst
  llvm/include/llvm/Support/ARMTargetParser.def
  llvm/lib/Target/ARM/ARM.td
  llvm/test/CodeGen/ARM/build-attributes.ll
  llvm/unittests/Support/TargetParserTest.cpp

Index: llvm/unittests/Support/TargetParserTest.cpp
===
--- llvm/unittests/Support/TargetParserTest.cpp
+++ llvm/unittests/Support/TargetParserTest.cpp
@@ -395,13 +395,19 @@
  ARM::AEK_FP | ARM::AEK_RAS | ARM::AEK_LOB |
  ARM::AEK_FP16,
  "8.1-M.Mainline"),
+ARMCPUTestParams("cortex-m85", "armv8.1-m.main",
+ "fp-armv8-fullfp16-d16",
+ ARM::AEK_HWDIVTHUMB | ARM::AEK_DSP | ARM::AEK_SIMD |
+ ARM::AEK_FP | ARM::AEK_RAS | ARM::AEK_LOB |
+ ARM::AEK_FP16 | ARM::AEK_PACBTI,
+ "8.1-M.Mainline"),
 ARMCPUTestParams("iwmmxt", "iwmmxt", "none", ARM::AEK_NONE, "iwmmxt"),
 ARMCPUTestParams("xscale", "xscale", "none", ARM::AEK_NONE, "xscale"),
 ARMCPUTestParams("swift", "armv7s", "neon-vfpv4",
  ARM::AEK_HWDIVARM | ARM::AEK_HWDIVTHUMB | ARM::AEK_DSP,
  "7-S")));
 
-static constexpr unsigned NumARMCPUArchs = 88;
+static constexpr unsigned NumARMCPUArchs = 89;
 
 TEST(TargetParserTest, testARMCPUArchList) {
   SmallVector List;
Index: llvm/test/CodeGen/ARM/build-attributes.ll
===
--- llvm/test/CodeGen/ARM/build-attributes.ll
+++ llvm/test/CodeGen/ARM/build-attributes.ll
@@ -235,6 +235,7 @@
 ; RUN: llc < %s -mtriple=thumbv8.1m.main-none-none-eabi -mattr=+mve.fp | FileCheck %s --check-prefix=ARMv81M-MAIN-MVEFP
 ; RUN: llc < %s -mtriple=thumbv8.1m.main-none-none-eabi -mattr=+pacbti | FileCheck %s --check-prefix=ARMv81M-MAIN-PACBTI
 ; RUN: llc < %s -mtriple=arm-none-none-eabi -mcpu=cortex-m55 | FileCheck %s --check-prefix=CORTEX-M55
+; RUN: llc < %s -mtriple=arm-none-none-eabi -mcpu=cortex-m85 | FileCheck %s --check-prefix=CORTEX-M85
 
 ; CPU-SUPPORTED-NOT: is not a recognized processor for this target
 
@@ -1748,6 +1749,19 @@
 ; CORTEX-M55: .eabi_attribute 38, 1
 ; CORTEX-M55: .eabi_attribute 14, 0
 
+; CORTEX-M85: .cpu cortex-m85
+; CORTEX-M85: .eabi_attribute 6, 21   @ Tag_CPU_arch
+; CORTEX-M85: .eabi_attribute 7, 77   @ Tag_CPU_arch_profile
+; CORTEX-M85: .eabi_attribute 8, 0@ Tag_ARM_ISA_use
+; CORTEX-M85: .eabi_attribute 9, 3@ Tag_THUMB_ISA_use
+; CORTEX-M85: .fpufpv5-d16
+; CORTEX-M85: .eabi_attribute 36, 1   @ Tag_FP_HP_extension
+; CORTEX-M85: .eabi_attribute 48, 2   @ Tag_MVE_arch
+; CORTEX-M85: .eabi_attribute 46, 1   @ Tag_DSP_extension
+; CORTEX-M85: .eabi_attribute 34, 1   @ Tag_CPU_unaligned_access
+; CORTEX-M85-NOT: .eabi_attribute 50
+; CORTEX-M85-NOT: .eabi_attribute 52
+
 define i32 @f(i64 %z) {
 ret i32 0
 }
Index: llvm/lib/Target/ARM/ARM.td
===
--- llvm/lib/Target/ARM/ARM.td
+++ llvm/lib/Target/ARM/ARM.td
@@ -1450,6 +1450,12 @@
  HasMVEFloatOps,
  FeatureFixCMSE_CVE_2021_35465]>;
 
+def : ProcessorModel<"cortex-m85", CortexM7Model,   [ARMv81mMainline,
+ FeatureDSP,
+ FeatureFPARMv8_D16,
+ FeatureUseMISched,
+ HasMVEFloatOps]>;
+
 def : ProcNoItin<"cortex-a32",   [ARMv8a,
  FeatureHWDivThumb,
  FeatureHWDivARM,
Index: llvm/include/llvm/Support/ARMTargetParser.def
===
--- llvm/include/llvm/Support/ARMTargetParser.def
+++ llvm/include/llvm/Support/ARMTargetParser.def
@@ -303,6 +303,9 @@
 ARM_CPU_NAME("cortex-m35p", ARMV8MMainline, FK_FPV5_SP_D16, false, ARM::AEK_DSP)
 ARM_CPU_NAME("cortex-m55", ARMV8_1MMainline, FK_FP_ARMV8_FULLFP16_D16, false,
  (ARM::AEK_DSP | ARM::AEK_SIMD | ARM::AEK_FP | ARM::AEK_FP16))
+ARM_CPU_NAME("cortex-m85", ARMV8_1MMainline, FK_FP_ARMV8_FULLFP16_D16, false,
+ (ARM::AEK_DSP | ARM::AEK_SIMD | ARM::AEK_FP | ARM::AEK_FP16 |
+  ARM::AEK_RAS | ARM::AEK_

[PATCH] D128415: [ARM] Add Support for Cortex-M85

[Sam Elliott via Phabricator via cfe-commits]

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

The other changes in this commit are to enable PACBTI by default, as agreed 
with stakeholders within Arm.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128415

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

[PATCH] D128415: [ARM] Add Support for Cortex-M85

[Sam Elliott via Phabricator via cfe-commits]

lenary added a comment.

In D128415#3628274 , @tschuett wrote:

> The Clang release notes indicate that PACBTI is off by default. In several 
> places, I can see PACBTI. Is the `ARM.td` missing something?

Nope, I should have re-checked the whole patch. Update coming to address this 
and @dmgreen's last comment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128415

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

[PATCH] D128415: [ARM] Add Support for Cortex-M85

[Sam Elliott via Phabricator via cfe-commits]

lenary updated this revision to Diff 442109.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128415

Files:
  clang/docs/ReleaseNotes.rst
  clang/test/CodeGen/arm-target-features.c
  clang/test/Driver/arm-cortex-cpus-2.c
  clang/test/Driver/arm-nofp-disabled-features.c
  clang/test/Driver/arm-nopacbti-disabled-features.c
  clang/test/Misc/target-invalid-cpu-note.c
  llvm/docs/ReleaseNotes.rst
  llvm/include/llvm/Support/ARMTargetParser.def
  llvm/lib/Target/ARM/ARM.td
  llvm/test/CodeGen/ARM/build-attributes.ll
  llvm/unittests/Support/TargetParserTest.cpp

Index: llvm/unittests/Support/TargetParserTest.cpp
===
--- llvm/unittests/Support/TargetParserTest.cpp
+++ llvm/unittests/Support/TargetParserTest.cpp
@@ -395,13 +395,19 @@
  ARM::AEK_FP | ARM::AEK_RAS | ARM::AEK_LOB |
  ARM::AEK_FP16,
  "8.1-M.Mainline"),
+ARMCPUTestParams("cortex-m85", "armv8.1-m.main",
+ "fp-armv8-fullfp16-d16",
+ ARM::AEK_HWDIVTHUMB | ARM::AEK_DSP | ARM::AEK_SIMD |
+ ARM::AEK_FP | ARM::AEK_RAS | ARM::AEK_LOB |
+ ARM::AEK_FP16 | ARM::AEK_PACBTI,
+ "8.1-M.Mainline"),
 ARMCPUTestParams("iwmmxt", "iwmmxt", "none", ARM::AEK_NONE, "iwmmxt"),
 ARMCPUTestParams("xscale", "xscale", "none", ARM::AEK_NONE, "xscale"),
 ARMCPUTestParams("swift", "armv7s", "neon-vfpv4",
  ARM::AEK_HWDIVARM | ARM::AEK_HWDIVTHUMB | ARM::AEK_DSP,
  "7-S")));
 
-static constexpr unsigned NumARMCPUArchs = 88;
+static constexpr unsigned NumARMCPUArchs = 89;
 
 TEST(TargetParserTest, testARMCPUArchList) {
   SmallVector List;
Index: llvm/test/CodeGen/ARM/build-attributes.ll
===
--- llvm/test/CodeGen/ARM/build-attributes.ll
+++ llvm/test/CodeGen/ARM/build-attributes.ll
@@ -235,6 +235,8 @@
 ; RUN: llc < %s -mtriple=thumbv8.1m.main-none-none-eabi -mattr=+mve.fp | FileCheck %s --check-prefix=ARMv81M-MAIN-MVEFP
 ; RUN: llc < %s -mtriple=thumbv8.1m.main-none-none-eabi -mattr=+pacbti | FileCheck %s --check-prefix=ARMv81M-MAIN-PACBTI
 ; RUN: llc < %s -mtriple=arm-none-none-eabi -mcpu=cortex-m55 | FileCheck %s --check-prefix=CORTEX-M55
+; RUN: llc < %s -mtriple=arm-none-none-eabi -mcpu=cortex-m85 | FileCheck %s --check-prefix=CORTEX-M85
+; RUN: llc < %s -mtriple=arm-none-none-eabi -mcpu=cortex-m85+nopacbti | FileCheck %s --check-prefix=CHECK-NO-PACBTI
 
 ; CPU-SUPPORTED-NOT: is not a recognized processor for this target
 
@@ -1748,6 +1750,23 @@
 ; CORTEX-M55: .eabi_attribute 38, 1
 ; CORTEX-M55: .eabi_attribute 14, 0
 
+; CORTEX-M85: .cpu cortex-m85
+; CORTEX-M85: .eabi_attribute 6, 21   @ Tag_CPU_arch
+; CORTEX-M85: .eabi_attribute 7, 77   @ Tag_CPU_arch_profile
+; CORTEX-M85: .eabi_attribute 8, 0@ Tag_ARM_ISA_use
+; CORTEX-M85: .eabi_attribute 9, 3@ Tag_THUMB_ISA_use
+; CORTEX-M85: .fpufpv5-d16
+; CORTEX-M85: .eabi_attribute 36, 1   @ Tag_FP_HP_extension
+; CORTEX-M85: .eabi_attribute 48, 2   @ Tag_MVE_arch
+; CORTEX-M85: .eabi_attribute 46, 1   @ Tag_DSP_extension
+; CORTEX-M85: .eabi_attribute 34, 1   @ Tag_CPU_unaligned_access
+; CORTEX-M85: .eabi_attribute 50, 2   @ Tag_PAC_extension
+; CORTEX-M85: .eabi_attribute 52, 2   @ Tag_BTI_extension
+
+; CHECK-NO-PACBTI-NOT: .eabi_attribute 50
+; CHECK-NO-PACBTI-NOT: .eabi_attribute 52
+
+
 define i32 @f(i64 %z) {
 ret i32 0
 }
Index: llvm/lib/Target/ARM/ARM.td
===
--- llvm/lib/Target/ARM/ARM.td
+++ llvm/lib/Target/ARM/ARM.td
@@ -1450,6 +1450,13 @@
  HasMVEFloatOps,
  FeatureFixCMSE_CVE_2021_35465]>;
 
+def : ProcessorModel<"cortex-m85", CortexM7Model,   [ARMv81mMainline,
+ FeatureDSP,
+ FeatureFPARMv8_D16,
+ FeaturePACBTI,
+ FeatureUseMISched,
+ HasMVEFloatOps]>;
+
 def : ProcNoItin<"cortex-a32",   [ARMv8a,
  FeatureHWDivThumb,
  FeatureHWDivARM,
Index: llvm/include/llvm/Support/ARMTargetParser.def
===
--- llvm/include/llvm/Support/ARMTargetParser.def
+++ llvm/include/llvm/Support/ARMTargetParser.def
@@ -303,6 +303,9 @@
 ARM_CPU_NAME("cortex-m35p", ARMV8MMainline, FK_FPV5_SP_

[PATCH] D128415: [ARM] Add Support for Cortex-M85

[Sam Elliott via Phabricator via cfe-commits]

lenary added a comment.

@tschuett I've corrected the patch based on your feedback. If you're happy, I'm 
going to land this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128415

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

[PATCH] D128415: [ARM] Add Support for Cortex-M85

[Sam Elliott 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 rG1666f09933ee: [ARM] Add Support for Cortex-M85 (authored by 
lenary).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128415

Files:
  clang/docs/ReleaseNotes.rst
  clang/test/CodeGen/arm-target-features.c
  clang/test/Driver/arm-cortex-cpus-2.c
  clang/test/Driver/arm-nofp-disabled-features.c
  clang/test/Driver/arm-nopacbti-disabled-features.c
  clang/test/Misc/target-invalid-cpu-note.c
  llvm/docs/ReleaseNotes.rst
  llvm/include/llvm/Support/ARMTargetParser.def
  llvm/lib/Target/ARM/ARM.td
  llvm/test/CodeGen/ARM/build-attributes.ll
  llvm/unittests/Support/TargetParserTest.cpp

Index: llvm/unittests/Support/TargetParserTest.cpp
===
--- llvm/unittests/Support/TargetParserTest.cpp
+++ llvm/unittests/Support/TargetParserTest.cpp
@@ -395,13 +395,19 @@
  ARM::AEK_FP | ARM::AEK_RAS | ARM::AEK_LOB |
  ARM::AEK_FP16,
  "8.1-M.Mainline"),
+ARMCPUTestParams("cortex-m85", "armv8.1-m.main",
+ "fp-armv8-fullfp16-d16",
+ ARM::AEK_HWDIVTHUMB | ARM::AEK_DSP | ARM::AEK_SIMD |
+ ARM::AEK_FP | ARM::AEK_RAS | ARM::AEK_LOB |
+ ARM::AEK_FP16 | ARM::AEK_PACBTI,
+ "8.1-M.Mainline"),
 ARMCPUTestParams("iwmmxt", "iwmmxt", "none", ARM::AEK_NONE, "iwmmxt"),
 ARMCPUTestParams("xscale", "xscale", "none", ARM::AEK_NONE, "xscale"),
 ARMCPUTestParams("swift", "armv7s", "neon-vfpv4",
  ARM::AEK_HWDIVARM | ARM::AEK_HWDIVTHUMB | ARM::AEK_DSP,
  "7-S")));
 
-static constexpr unsigned NumARMCPUArchs = 88;
+static constexpr unsigned NumARMCPUArchs = 89;
 
 TEST(TargetParserTest, testARMCPUArchList) {
   SmallVector List;
Index: llvm/test/CodeGen/ARM/build-attributes.ll
===
--- llvm/test/CodeGen/ARM/build-attributes.ll
+++ llvm/test/CodeGen/ARM/build-attributes.ll
@@ -235,6 +235,8 @@
 ; RUN: llc < %s -mtriple=thumbv8.1m.main-none-none-eabi -mattr=+mve.fp | FileCheck %s --check-prefix=ARMv81M-MAIN-MVEFP
 ; RUN: llc < %s -mtriple=thumbv8.1m.main-none-none-eabi -mattr=+pacbti | FileCheck %s --check-prefix=ARMv81M-MAIN-PACBTI
 ; RUN: llc < %s -mtriple=arm-none-none-eabi -mcpu=cortex-m55 | FileCheck %s --check-prefix=CORTEX-M55
+; RUN: llc < %s -mtriple=arm-none-none-eabi -mcpu=cortex-m85 | FileCheck %s --check-prefix=CORTEX-M85
+; RUN: llc < %s -mtriple=arm-none-none-eabi -mcpu=cortex-m85+nopacbti | FileCheck %s --check-prefix=CHECK-NO-PACBTI
 
 ; CPU-SUPPORTED-NOT: is not a recognized processor for this target
 
@@ -1748,6 +1750,23 @@
 ; CORTEX-M55: .eabi_attribute 38, 1
 ; CORTEX-M55: .eabi_attribute 14, 0
 
+; CORTEX-M85: .cpu cortex-m85
+; CORTEX-M85: .eabi_attribute 6, 21   @ Tag_CPU_arch
+; CORTEX-M85: .eabi_attribute 7, 77   @ Tag_CPU_arch_profile
+; CORTEX-M85: .eabi_attribute 8, 0@ Tag_ARM_ISA_use
+; CORTEX-M85: .eabi_attribute 9, 3@ Tag_THUMB_ISA_use
+; CORTEX-M85: .fpufpv5-d16
+; CORTEX-M85: .eabi_attribute 36, 1   @ Tag_FP_HP_extension
+; CORTEX-M85: .eabi_attribute 48, 2   @ Tag_MVE_arch
+; CORTEX-M85: .eabi_attribute 46, 1   @ Tag_DSP_extension
+; CORTEX-M85: .eabi_attribute 34, 1   @ Tag_CPU_unaligned_access
+; CORTEX-M85: .eabi_attribute 50, 2   @ Tag_PAC_extension
+; CORTEX-M85: .eabi_attribute 52, 2   @ Tag_BTI_extension
+
+; CHECK-NO-PACBTI-NOT: .eabi_attribute 50
+; CHECK-NO-PACBTI-NOT: .eabi_attribute 52
+
+
 define i32 @f(i64 %z) {
 ret i32 0
 }
Index: llvm/lib/Target/ARM/ARM.td
===
--- llvm/lib/Target/ARM/ARM.td
+++ llvm/lib/Target/ARM/ARM.td
@@ -1450,6 +1450,13 @@
  HasMVEFloatOps,
  FeatureFixCMSE_CVE_2021_35465]>;
 
+def : ProcessorModel<"cortex-m85", CortexM7Model,   [ARMv81mMainline,
+ FeatureDSP,
+ FeatureFPARMv8_D16,
+ FeaturePACBTI,
+ FeatureUseMISched,
+ HasMVEFloatOps]>;
+
 def : ProcNoItin<"cortex-a32",   [ARMv8a,
  FeatureHWDivThumb,
  FeatureHWDivARM,
Index: llvm/include/llvm/Support/ARMTargetParser.def
===
--

[PATCH] D128653: [PowerPC] Fix the check for scalar MASS conversion

[Sam Elliott via Phabricator via cfe-commits]

lenary added a comment.

You likely need `// REQUIRES: powerpc-registered-target` in the top of the 
test, as `-enable-ppc-gen-scalar-mass` is only present if the PowerPC target 
has been compiled into LLVM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128653

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

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

[Sam Elliott via Phabricator via cfe-commits]

lenary added a comment.

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

[Sam Elliott via Phabricator via cfe-commits]

lenary updated this revision to Diff 419202.
lenary added a comment.

- Rewrite pass in terms of ReachingDefAnalysis
- Split tests into separate commit, for ease of review.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119720

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Arch/ARM.cpp
  clang/test/Driver/arm-fix-cortex-a57-aes-1742098.c
  llvm/lib/CodeGen/RDFGraph.cpp
  llvm/lib/Target/ARM/ARM.h
  llvm/lib/Target/ARM/ARM.td
  llvm/lib/Target/ARM/ARMFixCortexA57AES1742098Pass.cpp
  llvm/lib/Target/ARM/ARMSubtarget.h
  llvm/lib/Target/ARM/ARMTargetMachine.cpp
  llvm/lib/Target/ARM/CMakeLists.txt
  llvm/test/CodeGen/ARM/O3-pipeline.ll
  llvm/test/CodeGen/ARM/aes-erratum-fix.ll

Index: llvm/test/CodeGen/ARM/aes-erratum-fix.ll
===
--- llvm/test/CodeGen/ARM/aes-erratum-fix.ll
+++ llvm/test/CodeGen/ARM/aes-erratum-fix.ll
@@ -24,6 +24,7 @@
 ; CHECK-FIX:   @ %bb.0:
 ; CHECK-FIX-NEXT:vld1.64 {d16, d17}, [r0]
 ; CHECK-FIX-NEXT:vmov.i32 q9, #0x0
+; CHECK-FIX-NEXT:vorr q9, q9, q9
 ; CHECK-FIX-NEXT:aese.8 q9, q8
 ; CHECK-FIX-NEXT:aesmc.8 q8, q9
 ; CHECK-FIX-NEXT:vst1.64 {d16, d17}, [r0]
@@ -55,6 +56,8 @@
 define arm_aapcs_vfpcc <16 x i8> @aese_once_via_val(<16 x i8> %0, <16 x i8> %1) nounwind {
 ; CHECK-FIX-LABEL: aese_once_via_val:
 ; CHECK-FIX:   @ %bb.0:
+; CHECK-FIX-NEXT:vorr q1, q1, q1
+; CHECK-FIX-NEXT:vorr q0, q0, q0
 ; CHECK-FIX-NEXT:aese.8 q1, q0
 ; CHECK-FIX-NEXT:aesmc.8 q0, q1
 ; CHECK-FIX-NEXT:bx lr
@@ -91,6 +94,9 @@
 define arm_aapcs_vfpcc <16 x i8> @aese_twice_via_val(<16 x i8> %0, <16 x i8> %1) nounwind {
 ; CHECK-FIX-LABEL: aese_twice_via_val:
 ; CHECK-FIX:   @ %bb.0:
+; CHECK-FIX-NEXT:vorr q1, q1, q1
+; CHECK-FIX-NEXT:vorr q0, q0, q0
+; CHECK-FIX-NEXT:vorr q0, q0, q0
 ; CHECK-FIX-NEXT:aese.8 q1, q0
 ; CHECK-FIX-NEXT:aesmc.8 q8, q1
 ; CHECK-FIX-NEXT:aese.8 q8, q0
@@ -154,6 +160,8 @@
 define arm_aapcs_vfpcc <16 x i8> @aese_loop_via_val(i32 %0, <16 x i8> %1, <16 x i8> %2) nounwind {
 ; CHECK-FIX-LABEL: aese_loop_via_val:
 ; CHECK-FIX:   @ %bb.0:
+; CHECK-FIX-NEXT:vorr q1, q1, q1
+; CHECK-FIX-NEXT:vorr q0, q0, q0
 ; CHECK-FIX-NEXT:cmp r0, #0
 ; CHECK-FIX-NEXT:beq .LBB6_2
 ; CHECK-FIX-NEXT:  .LBB6_1: @ =>This Inner Loop Header: Depth=1
@@ -186,6 +194,7 @@
 ; CHECK-FIX:   @ %bb.0:
 ; CHECK-FIX-NEXT:vld1.8 {d0[0]}, [r0]
 ; CHECK-FIX-NEXT:vld1.64 {d16, d17}, [r1]
+; CHECK-FIX-NEXT:vorr q0, q0, q0
 ; CHECK-FIX-NEXT:aese.8 q8, q0
 ; CHECK-FIX-NEXT:aesmc.8 q8, q8
 ; CHECK-FIX-NEXT:vst1.64 {d16, d17}, [r1]
@@ -202,8 +211,10 @@
 define arm_aapcs_vfpcc void @aese_set8_via_val(i8 zeroext %0, <16 x i8> %1, <16 x i8>* %2) nounwind {
 ; CHECK-FIX-LABEL: aese_set8_via_val:
 ; CHECK-FIX:   @ %bb.0:
+; CHECK-FIX-NEXT:vorr q0, q0, q0
 ; CHECK-FIX-NEXT:vld1.64 {d16, d17}, [r1]
 ; CHECK-FIX-NEXT:vmov.8 d16[0], r0
+; CHECK-FIX-NEXT:vorr q8, q8, q8
 ; CHECK-FIX-NEXT:aese.8 q8, q0
 ; CHECK-FIX-NEXT:aesmc.8 q8, q8
 ; CHECK-FIX-NEXT:vst1.64 {d16, d17}, [r1]
@@ -225,6 +236,7 @@
 ; CHECK-FIX-NEXT:  @ %bb.1:
 ; CHECK-FIX-NEXT:vld1.8 {d0[0]}, [r1]
 ; CHECK-FIX-NEXT:  .LBB9_2:
+; CHECK-FIX-NEXT:vorr q0, q0, q0
 ; CHECK-FIX-NEXT:aese.8 q8, q0
 ; CHECK-FIX-NEXT:aesmc.8 q8, q8
 ; CHECK-FIX-NEXT:vst1.64 {d16, d17}, [r2]
@@ -248,12 +260,14 @@
 define arm_aapcs_vfpcc void @aese_set8_cond_via_val(i1 zeroext %0, i8 zeroext %1, <16 x i8> %2, <16 x i8>* %3) nounwind {
 ; CHECK-FIX-LABEL: aese_set8_cond_via_val:
 ; CHECK-FIX:   @ %bb.0:
+; CHECK-FIX-NEXT:vorr q0, q0, q0
 ; CHECK-FIX-NEXT:vld1.64 {d16, d17}, [r2]
 ; CHECK-FIX-NEXT:cmp r0, #0
 ; CHECK-FIX-NEXT:beq .LBB10_2
 ; CHECK-FIX-NEXT:  @ %bb.1:
 ; CHECK-FIX-NEXT:vmov.8 d16[0], r1
 ; CHECK-FIX-NEXT:  .LBB10_2: @ %select.end
+; CHECK-FIX-NEXT:vorr q8, q8, q8
 ; CHECK-FIX-NEXT:aese.8 q8, q0
 ; CHECK-FIX-NEXT:aesmc.8 q8, q8
 ; CHECK-FIX-NEXT:vst1.64 {d16, d17}, [r2]
@@ -276,6 +290,7 @@
 ; CHECK-FIX-NEXT:vld1.8 {d0[0]}, [r1]
 ; CHECK-FIX-NEXT:vld1.64 {d16, d17}, [r2]
 ; CHECK-FIX-NEXT:  .LBB11_2: @ =>This Inner Loop Header: Depth=1
+; CHECK-FIX-NEXT:vorr q0, q0, q0
 ; CHECK-FIX-NEXT:aese.8 q8, q0
 ; CHECK-FIX-NEXT:subs r0, r0, #1
 ; CHECK-FIX-NEXT:aesmc.8 q8, q8
@@ -318,6 +333,7 @@
 ; CHECK-FIX-NEXT:vmov.8 d0[0], r1
 ; CHECK-FIX-NEXT:vld1.64 {d16, d17}, [r2]
 ; CHECK-FIX-NEXT:  .LBB12_2: @ =>This Inner Loop Header: Depth=1
+; CHECK-FIX-NEXT:vorr q0, q0, q0
 ; CHECK-FIX-NEXT:aese.8 q8, q0
 ; CHECK-FIX-NEXT:subs r0, r0, #1
 ; CHECK-FIX-NEXT:aesmc.8 q8, q8
@@ -355,6 +371,7 @@
 ; CHECK-FIX:   @ %bb.0:
 ; CHECK-FIX-NEXT:vld1.16 {d0[0]}, [r0:16]
 ; CHECK-FIX-NEXT:vld1.64 {d16, d17}, [r1]
+; CHECK-FIX-NEXT:vorr q0, q0, q0
 ; CHECK-FIX-NEXT:ae

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

[Sam Elliott via Phabricator via cfe-commits]

lenary removed a reviewer: kparzysz.
lenary added a comment.

@kparzysz I have rewritten this to avoid using RDFGraph, so I don't think this 
needs you to review it any more.




Comment at: llvm/test/CodeGen/ARM/aes-erratum-fix.ll:49
+
+define <16 x i8> @aese_once_via_val(<16 x i8> %0, <16 x i8> %1) nounwind {
+; CHECK-FIX-NOSCHED-LABEL: aese_once_via_val:

dmgreen wrote:
> Adding arm_aapcs_vfpcc will make the function "hardfp", which might be useful 
> for testing inputs from argument that don't need to be passed via gpr regs.
Yeah, seems I was too zealous with removing some of these attributes. 


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

[Sam Elliott via Phabricator via cfe-commits]

lenary planned changes to this revision.
lenary added inline comments.



Comment at: llvm/lib/CodeGen/RDFGraph.cpp:1096
 RegisterRef RR = PDA.Addr->getRegRef(*this);
-#ifndef NDEBUG
-// Assert if the register is defined in two or more unrelated defs.
-// This could happen if there are two or more def operands defining it.
-if (!Defined.insert(RR.Reg).second) {
-  MachineInstr *MI = NodeAddr(IA).Addr->getCode();
-  dbgs() << "Multiple definitions of register: "
- << Print(RR, *this) << " in\n  " << *MI << "in "
- << printMBBReference(*MI->getParent()) << '\n';
-  llvm_unreachable(nullptr);
-}
-#endif
+// #ifndef NDEBUG
+// // Assert if the register is defined in two or more unrelated defs.

I forgot to remove these comments, will update shortly.



Comment at: llvm/lib/Target/ARM/ARMSubtarget.h:543
   unsigned getGPRAllocationOrder(const MachineFunction &MF) const;
+
 };

I missed this, will update again shortly.


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

[Sam Elliott via Phabricator via cfe-commits]

lenary updated this revision to Diff 419208.
lenary added a comment.

- Remove whitespace change in ARMSubtarget
- Remove commented-out debug lines in RDFGraph


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119720

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Arch/ARM.cpp
  clang/test/Driver/arm-fix-cortex-a57-aes-1742098.c
  llvm/lib/Target/ARM/ARM.h
  llvm/lib/Target/ARM/ARM.td
  llvm/lib/Target/ARM/ARMFixCortexA57AES1742098Pass.cpp
  llvm/lib/Target/ARM/ARMTargetMachine.cpp
  llvm/lib/Target/ARM/CMakeLists.txt
  llvm/test/CodeGen/ARM/O3-pipeline.ll
  llvm/test/CodeGen/ARM/aes-erratum-fix.ll

Index: llvm/test/CodeGen/ARM/aes-erratum-fix.ll
===
--- llvm/test/CodeGen/ARM/aes-erratum-fix.ll
+++ llvm/test/CodeGen/ARM/aes-erratum-fix.ll
@@ -24,6 +24,7 @@
 ; CHECK-FIX:   @ %bb.0:
 ; CHECK-FIX-NEXT:vld1.64 {d16, d17}, [r0]
 ; CHECK-FIX-NEXT:vmov.i32 q9, #0x0
+; CHECK-FIX-NEXT:vorr q9, q9, q9
 ; CHECK-FIX-NEXT:aese.8 q9, q8
 ; CHECK-FIX-NEXT:aesmc.8 q8, q9
 ; CHECK-FIX-NEXT:vst1.64 {d16, d17}, [r0]
@@ -55,6 +56,8 @@
 define arm_aapcs_vfpcc <16 x i8> @aese_once_via_val(<16 x i8> %0, <16 x i8> %1) nounwind {
 ; CHECK-FIX-LABEL: aese_once_via_val:
 ; CHECK-FIX:   @ %bb.0:
+; CHECK-FIX-NEXT:vorr q1, q1, q1
+; CHECK-FIX-NEXT:vorr q0, q0, q0
 ; CHECK-FIX-NEXT:aese.8 q1, q0
 ; CHECK-FIX-NEXT:aesmc.8 q0, q1
 ; CHECK-FIX-NEXT:bx lr
@@ -91,6 +94,9 @@
 define arm_aapcs_vfpcc <16 x i8> @aese_twice_via_val(<16 x i8> %0, <16 x i8> %1) nounwind {
 ; CHECK-FIX-LABEL: aese_twice_via_val:
 ; CHECK-FIX:   @ %bb.0:
+; CHECK-FIX-NEXT:vorr q1, q1, q1
+; CHECK-FIX-NEXT:vorr q0, q0, q0
+; CHECK-FIX-NEXT:vorr q0, q0, q0
 ; CHECK-FIX-NEXT:aese.8 q1, q0
 ; CHECK-FIX-NEXT:aesmc.8 q8, q1
 ; CHECK-FIX-NEXT:aese.8 q8, q0
@@ -154,6 +160,8 @@
 define arm_aapcs_vfpcc <16 x i8> @aese_loop_via_val(i32 %0, <16 x i8> %1, <16 x i8> %2) nounwind {
 ; CHECK-FIX-LABEL: aese_loop_via_val:
 ; CHECK-FIX:   @ %bb.0:
+; CHECK-FIX-NEXT:vorr q1, q1, q1
+; CHECK-FIX-NEXT:vorr q0, q0, q0
 ; CHECK-FIX-NEXT:cmp r0, #0
 ; CHECK-FIX-NEXT:beq .LBB6_2
 ; CHECK-FIX-NEXT:  .LBB6_1: @ =>This Inner Loop Header: Depth=1
@@ -186,6 +194,7 @@
 ; CHECK-FIX:   @ %bb.0:
 ; CHECK-FIX-NEXT:vld1.8 {d0[0]}, [r0]
 ; CHECK-FIX-NEXT:vld1.64 {d16, d17}, [r1]
+; CHECK-FIX-NEXT:vorr q0, q0, q0
 ; CHECK-FIX-NEXT:aese.8 q8, q0
 ; CHECK-FIX-NEXT:aesmc.8 q8, q8
 ; CHECK-FIX-NEXT:vst1.64 {d16, d17}, [r1]
@@ -202,8 +211,10 @@
 define arm_aapcs_vfpcc void @aese_set8_via_val(i8 zeroext %0, <16 x i8> %1, <16 x i8>* %2) nounwind {
 ; CHECK-FIX-LABEL: aese_set8_via_val:
 ; CHECK-FIX:   @ %bb.0:
+; CHECK-FIX-NEXT:vorr q0, q0, q0
 ; CHECK-FIX-NEXT:vld1.64 {d16, d17}, [r1]
 ; CHECK-FIX-NEXT:vmov.8 d16[0], r0
+; CHECK-FIX-NEXT:vorr q8, q8, q8
 ; CHECK-FIX-NEXT:aese.8 q8, q0
 ; CHECK-FIX-NEXT:aesmc.8 q8, q8
 ; CHECK-FIX-NEXT:vst1.64 {d16, d17}, [r1]
@@ -225,6 +236,7 @@
 ; CHECK-FIX-NEXT:  @ %bb.1:
 ; CHECK-FIX-NEXT:vld1.8 {d0[0]}, [r1]
 ; CHECK-FIX-NEXT:  .LBB9_2:
+; CHECK-FIX-NEXT:vorr q0, q0, q0
 ; CHECK-FIX-NEXT:aese.8 q8, q0
 ; CHECK-FIX-NEXT:aesmc.8 q8, q8
 ; CHECK-FIX-NEXT:vst1.64 {d16, d17}, [r2]
@@ -248,12 +260,14 @@
 define arm_aapcs_vfpcc void @aese_set8_cond_via_val(i1 zeroext %0, i8 zeroext %1, <16 x i8> %2, <16 x i8>* %3) nounwind {
 ; CHECK-FIX-LABEL: aese_set8_cond_via_val:
 ; CHECK-FIX:   @ %bb.0:
+; CHECK-FIX-NEXT:vorr q0, q0, q0
 ; CHECK-FIX-NEXT:vld1.64 {d16, d17}, [r2]
 ; CHECK-FIX-NEXT:cmp r0, #0
 ; CHECK-FIX-NEXT:beq .LBB10_2
 ; CHECK-FIX-NEXT:  @ %bb.1:
 ; CHECK-FIX-NEXT:vmov.8 d16[0], r1
 ; CHECK-FIX-NEXT:  .LBB10_2: @ %select.end
+; CHECK-FIX-NEXT:vorr q8, q8, q8
 ; CHECK-FIX-NEXT:aese.8 q8, q0
 ; CHECK-FIX-NEXT:aesmc.8 q8, q8
 ; CHECK-FIX-NEXT:vst1.64 {d16, d17}, [r2]
@@ -276,6 +290,7 @@
 ; CHECK-FIX-NEXT:vld1.8 {d0[0]}, [r1]
 ; CHECK-FIX-NEXT:vld1.64 {d16, d17}, [r2]
 ; CHECK-FIX-NEXT:  .LBB11_2: @ =>This Inner Loop Header: Depth=1
+; CHECK-FIX-NEXT:vorr q0, q0, q0
 ; CHECK-FIX-NEXT:aese.8 q8, q0
 ; CHECK-FIX-NEXT:subs r0, r0, #1
 ; CHECK-FIX-NEXT:aesmc.8 q8, q8
@@ -318,6 +333,7 @@
 ; CHECK-FIX-NEXT:vmov.8 d0[0], r1
 ; CHECK-FIX-NEXT:vld1.64 {d16, d17}, [r2]
 ; CHECK-FIX-NEXT:  .LBB12_2: @ =>This Inner Loop Header: Depth=1
+; CHECK-FIX-NEXT:vorr q0, q0, q0
 ; CHECK-FIX-NEXT:aese.8 q8, q0
 ; CHECK-FIX-NEXT:subs r0, r0, #1
 ; CHECK-FIX-NEXT:aesmc.8 q8, q8
@@ -355,6 +371,7 @@
 ; CHECK-FIX:   @ %bb.0:
 ; CHECK-FIX-NEXT:vld1.16 {d0[0]}, [r0:16]
 ; CHECK-FIX-NEXT:vld1.64 {d16, d17}, [r1]
+; CHECK-FIX-NEXT:vorr q0, q0, q0
 ; CHECK-FIX-NEXT:aese.8 q8, q0
 ; CHECK-FIX-NEXT:aesmc.8 q8, q8
 ; CHECK-FIX-NEXT:vst1.64 {d1

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

[Sam Elliott via Phabricator via cfe-commits]

lenary added a comment.

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




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

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

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



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

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



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

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

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

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

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

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

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

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

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







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

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

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


Repository:
  rG LLVM Github Monorepo

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

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

[Sam Elliott via Phabricator via cfe-commits]

lenary marked 6 inline comments as done.
lenary added a comment.

A few comments before I post the next version of the patch.




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

lenary wrote:
> dmgreen wrote:
> > Can/should all these use findFirstPredOperandIdx?
> > 
> > And is it worth checking for more instruction? Anything that sets a Q 
> > register, or is that too broad?
> `findFirstPredOperandIdx` doesn't work as lots of these instructions are not 
> marked `isPredicable` in the tablegen. I'm not sure if we want to solve that 
> in this work, or as a follow-up (I'd lean towards follow-up).
> 
> I believe "anything that sets a Q register" is too broad, as we model 
> subregister insertion as setting the the whole register in LLVM, but I'm not 
> sure that micro-architecturally they are actually doing that. This is why 
> I've tried to only add 64- and 128-bit setting instructions rather than ones 
> that are less wide. Originally I also included the `VMOVv2f32` instructions 
> that are now at the bottom of this switch, but I felt that might have been 
> too risky.
I'm wrong about this. The tablegen `isPredicable` is an override, other code 
might also set `isPredicable` to true, so I think `findFirstPredOperandIdx` 
should work too.



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

dmgreen wrote:
> Are these vmov of an immediate? Are they not safe?
> 
> I was expecting it to be the lanes sets (VSETLNi8) and other scalar 
> instructions that were unsafe.
I have received the information on what is safe and what is not, and the next 
version of the patch will have this correct.



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

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

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

[Sam Elliott via Phabricator via cfe-commits]

lenary updated this revision to Diff 421193.
lenary marked 2 inline comments as done.
lenary added a comment.

- Updated set of safe instructions
- Address reviewer feedback, including reordering passes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119720

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Arch/ARM.cpp
  clang/test/Driver/arm-fix-cortex-a57-aes-1742098.c
  llvm/lib/Target/ARM/ARM.h
  llvm/lib/Target/ARM/ARM.td
  llvm/lib/Target/ARM/ARMFixCortexA57AES1742098Pass.cpp
  llvm/lib/Target/ARM/ARMTargetMachine.cpp
  llvm/lib/Target/ARM/CMakeLists.txt
  llvm/test/CodeGen/ARM/O3-pipeline.ll
  llvm/test/CodeGen/ARM/aes-erratum-fix.ll

Index: llvm/test/CodeGen/ARM/aes-erratum-fix.ll
===
--- llvm/test/CodeGen/ARM/aes-erratum-fix.ll
+++ llvm/test/CodeGen/ARM/aes-erratum-fix.ll
@@ -47,6 +47,7 @@
 ; CHECK-FIX-NEXT:push {r4, lr}
 ; CHECK-FIX-NEXT:mov r4, r0
 ; CHECK-FIX-NEXT:bl get_input
+; CHECK-FIX-NEXT:vorr q0, q0, q0
 ; CHECK-FIX-NEXT:vld1.64 {d16, d17}, [r4]
 ; CHECK-FIX-NEXT:aese.8 q0, q8
 ; CHECK-FIX-NEXT:aesmc.8 q8, q0
@@ -67,6 +68,7 @@
 ; CHECK-FIX-NEXT:push {r4, lr}
 ; CHECK-FIX-NEXT:mov r4, r0
 ; CHECK-FIX-NEXT:bl get_inputf16
+; CHECK-FIX-NEXT:vorr q0, q0, q0
 ; CHECK-FIX-NEXT:vld1.64 {d16, d17}, [r4]
 ; CHECK-FIX-NEXT:aese.8 q0, q8
 ; CHECK-FIX-NEXT:aesmc.8 q8, q0
@@ -87,6 +89,7 @@
 ; CHECK-FIX-NEXT:push {r4, lr}
 ; CHECK-FIX-NEXT:mov r4, r0
 ; CHECK-FIX-NEXT:bl get_inputf32
+; CHECK-FIX-NEXT:vorr q0, q0, q0
 ; CHECK-FIX-NEXT:vld1.64 {d16, d17}, [r4]
 ; CHECK-FIX-NEXT:aese.8 q0, q8
 ; CHECK-FIX-NEXT:aesmc.8 q8, q0
@@ -120,6 +123,8 @@
 define arm_aapcs_vfpcc <16 x i8> @aese_once_via_val(<16 x i8> %0, <16 x i8> %1) nounwind {
 ; CHECK-FIX-LABEL: aese_once_via_val:
 ; CHECK-FIX:   @ %bb.0:
+; CHECK-FIX-NEXT:vorr q1, q1, q1
+; CHECK-FIX-NEXT:vorr q0, q0, q0
 ; CHECK-FIX-NEXT:aese.8 q1, q0
 ; CHECK-FIX-NEXT:aesmc.8 q0, q1
 ; CHECK-FIX-NEXT:bx lr
@@ -156,6 +161,9 @@
 define arm_aapcs_vfpcc <16 x i8> @aese_twice_via_val(<16 x i8> %0, <16 x i8> %1) nounwind {
 ; CHECK-FIX-LABEL: aese_twice_via_val:
 ; CHECK-FIX:   @ %bb.0:
+; CHECK-FIX-NEXT:vorr q1, q1, q1
+; CHECK-FIX-NEXT:vorr q0, q0, q0
+; CHECK-FIX-NEXT:vorr q0, q0, q0
 ; CHECK-FIX-NEXT:aese.8 q1, q0
 ; CHECK-FIX-NEXT:aesmc.8 q8, q1
 ; CHECK-FIX-NEXT:aese.8 q8, q0
@@ -219,6 +227,8 @@
 define arm_aapcs_vfpcc <16 x i8> @aese_loop_via_val(i32 %0, <16 x i8> %1, <16 x i8> %2) nounwind {
 ; CHECK-FIX-LABEL: aese_loop_via_val:
 ; CHECK-FIX:   @ %bb.0:
+; CHECK-FIX-NEXT:vorr q1, q1, q1
+; CHECK-FIX-NEXT:vorr q0, q0, q0
 ; CHECK-FIX-NEXT:cmp r0, #0
 ; CHECK-FIX-NEXT:beq .LBB9_2
 ; CHECK-FIX-NEXT:  .LBB9_1: @ =>This Inner Loop Header: Depth=1
@@ -249,6 +259,7 @@
 define arm_aapcs_vfpcc void @aese_set8_via_ptr(i8* %0, <16 x i8> %1, <16 x i8>* %2) nounwind {
 ; CHECK-FIX-NOSCHED-LABEL: aese_set8_via_ptr:
 ; CHECK-FIX-NOSCHED:   @ %bb.0:
+; CHECK-FIX-NOSCHED-NEXT:vorr q0, q0, q0
 ; CHECK-FIX-NOSCHED-NEXT:ldrb r0, [r0]
 ; CHECK-FIX-NOSCHED-NEXT:vld1.64 {d16, d17}, [r1]
 ; CHECK-FIX-NOSCHED-NEXT:vmov.8 d0[0], r0
@@ -260,6 +271,7 @@
 ;
 ; CHECK-CORTEX-FIX-LABEL: aese_set8_via_ptr:
 ; CHECK-CORTEX-FIX:   @ %bb.0:
+; CHECK-CORTEX-FIX-NEXT:vorr q0, q0, q0
 ; CHECK-CORTEX-FIX-NEXT:vld1.64 {d16, d17}, [r1]
 ; CHECK-CORTEX-FIX-NEXT:ldrb r0, [r0]
 ; CHECK-CORTEX-FIX-NEXT:vmov.8 d0[0], r0
@@ -281,6 +293,7 @@
 define arm_aapcs_vfpcc void @aese_set8_via_val(i8 zeroext %0, <16 x i8> %1, <16 x i8>* %2) nounwind {
 ; CHECK-FIX-LABEL: aese_set8_via_val:
 ; CHECK-FIX:   @ %bb.0:
+; CHECK-FIX-NEXT:vorr q0, q0, q0
 ; CHECK-FIX-NEXT:vld1.64 {d16, d17}, [r1]
 ; CHECK-FIX-NEXT:vmov.8 d0[0], r0
 ; CHECK-FIX-NEXT:vmov.8 d16[0], r0
@@ -300,6 +313,7 @@
 define arm_aapcs_vfpcc void @aese_set8_cond_via_ptr(i1 zeroext %0, i8* %1, <16 x i8> %2, <16 x i8>* %3) nounwind {
 ; CHECK-FIX-LABEL: aese_set8_cond_via_ptr:
 ; CHECK-FIX:   @ %bb.0:
+; CHECK-FIX-NEXT:vorr q0, q0, q0
 ; CHECK-FIX-NEXT:cmp r0, #0
 ; CHECK-FIX-NEXT:beq .LBB12_2
 ; CHECK-FIX-NEXT:  @ %bb.1:
@@ -351,6 +365,7 @@
 define arm_aapcs_vfpcc void @aese_set8_cond_via_val(i1 zeroext %0, i8 zeroext %1, <16 x i8> %2, <16 x i8>* %3) nounwind {
 ; CHECK-FIX-LABEL: aese_set8_cond_via_val:
 ; CHECK-FIX:   @ %bb.0:
+; CHECK-FIX-NEXT:vorr q0, q0, q0
 ; CHECK-FIX-NEXT:vld1.64 {d16, d17}, [r2]
 ; CHECK-FIX-NEXT:cmp r0, #0
 ; CHECK-FIX-NEXT:beq .LBB13_2
@@ -380,6 +395,7 @@
 define arm_aapcs_vfpcc void @aese_set8_loop_via_ptr(i32 %0, i8* %1, <16 x i8> %2, <16 x i8>* %3) nounwind {
 ; CHECK-FIX-LABEL: aese_set8_loop_via_ptr:
 ; CHECK-FIX:   @ %bb.0:
+; CHECK-FIX-NEXT:vorr q0, q0, q0
 ; CHECK-FIX-NEXT:ldrb r1, 

[PATCH] D119301: [AArch64][ARM] add -Wunaligned-access only for clang

[Sam Elliott via Phabricator via cfe-commits]

lenary added a comment.

I don't fully understand the reasoning for the patch, and you haven't really 
explained it. I think what you are saying is that the `IsAux` argument to 
`getTargetFeatures` should be considered because it's `true` for offloading to 
another compiler, but I don't understand why we think the offload compiler is 
not clang-compatible, as the features added in `getAArch64TargetFeatures` and 
`getARMTargetFeatures` (and passed to the offloaded compiler) use the 
LLVM-specific internal names. I would like you to explain this further, and to 
document what `IsAux` implies in the code.

That all said code given here is not equivalent to the feature as originally 
landed, because you've only re-added the `-Wunaligned-access` flag for the 
AArch64 target - please make the same addition in `Clang::AddARMTargetArgs` as 
we want the warning on both. I think this also makes `CmdArgs` unused in both 
`aarch64::getAArch64TargetFeatures` and `arm::getARMTargetFeatures` as I think 
that you don't actually want us adding to `CmdArgs` in those functions (again, 
I hope the full reasoning behind this will be explained when you explain why 
the whole patch is necessary).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119301

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

[PATCH] D119301: [AArch64][ARM] add -Wunaligned-access only for clang

[Sam Elliott via Phabricator via cfe-commits]

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

Ah, I see.

Please can you also remove the `CmdArgs` parameter from 
`arm::getARMTargetFeatures` when you commit this? This would be useful, along 
with a comment at the top of `getTargetFeatures` to discourage targets adding 
to or using `CmdArgs` in their `getTargetFeatures` functions.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119301

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

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

[Sam Elliott via Phabricator via cfe-commits]

lenary added a comment.

@kparzysz I've tagged you due to the changes in RDFGraph, which I believe you 
are the owner of. The asserts are hit in 
`llvm/test/CodeGen/ARM/inlineasm-error-t-toofewregs.ll` - the Register 
Allocator Chokes on the test due to not having enough registers for the inline 
assembly. `llc` then reports the error, but continues running passes (including 
this one), which then core dumps on the asserts I've removed (The first one hit 
is in `DataFlowGraph::buildStmt`). When I run the same testcase with 
`-verify-machineinstrs`, there is no verification error, but the verifier seems 
to have decided that analysing physical reg liveness is too hard (see e.g. 
`MachineVerifier.cpp` line 2990). I haven't worked out how else to prevent this 
pass running if the register allocation is known to be wrong, without analysing 
every instruction before building the RDFGraph. I wasn't sure the right 
approach, so I wanted to post the patch and then discuss it, rather than the 
other way around (so you can see the testcase).


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

[Sam Elliott via Phabricator via cfe-commits]

lenary added a comment.

Ack to all the comment clarifications, will update patch with those soon 
(probably tomorrow).




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

dmgreen wrote:
> Perhaps add these, if they are safe:
> VBICd/q
> VBICi's, VORRi's
> VBIF/VBIT/VBSL/VBSP
> VCEQ/VCNE/etc?
> VDUP? VEXT?
> VMVN imm equivalents of VMOV's
> VREV's?
> VSHL's, VSHR's?
> I'm not sure if they will be very useful, but they are the kind of 
> instructions that may come up in aes algorithms.
I'm very keen to avoid scope-creep on this patch, so I'm going to push back on 
this comment.

We know this list as given is safe (and have had internal confirmation). I've 
sent a new email internally with your list of instructions, to find out of 
they're safe too, but I'd like any answer to that to be part of a follow-up 
patch rather than blocking this patch for yet longer.

I believe what I have today is correct, even if the list is not optimal for all 
expected AES code sequences.


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

[Sam Elliott via Phabricator via cfe-commits]

lenary updated this revision to Diff 428975.
lenary added a comment.
Herald added subscribers: cfe-commits, MaskRay, hiraditya, mgorny.
Herald added a project: clang.

- Rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122747

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Arch/ARM.cpp
  clang/test/Driver/arm-fix-cortex-a57-aes-1742098.c
  llvm/lib/Target/ARM/ARM.h
  llvm/lib/Target/ARM/ARM.td
  llvm/lib/Target/ARM/ARMFixCortexA57AES1742098Pass.cpp
  llvm/lib/Target/ARM/ARMTargetMachine.cpp
  llvm/lib/Target/ARM/CMakeLists.txt
  llvm/test/CodeGen/ARM/O3-pipeline.ll
  llvm/test/CodeGen/ARM/aes-erratum-fix.ll

Index: llvm/test/CodeGen/ARM/aes-erratum-fix.ll
===
--- llvm/test/CodeGen/ARM/aes-erratum-fix.ll
+++ llvm/test/CodeGen/ARM/aes-erratum-fix.ll
@@ -47,6 +47,7 @@
 ; CHECK-FIX-NEXT:push {r4, lr}
 ; CHECK-FIX-NEXT:mov r4, r0
 ; CHECK-FIX-NEXT:bl get_input
+; CHECK-FIX-NEXT:vorr q0, q0, q0
 ; CHECK-FIX-NEXT:vld1.64 {d16, d17}, [r4]
 ; CHECK-FIX-NEXT:aese.8 q0, q8
 ; CHECK-FIX-NEXT:aesmc.8 q8, q0
@@ -67,6 +68,7 @@
 ; CHECK-FIX-NEXT:push {r4, lr}
 ; CHECK-FIX-NEXT:mov r4, r0
 ; CHECK-FIX-NEXT:bl get_inputf16
+; CHECK-FIX-NEXT:vorr q0, q0, q0
 ; CHECK-FIX-NEXT:vld1.64 {d16, d17}, [r4]
 ; CHECK-FIX-NEXT:aese.8 q0, q8
 ; CHECK-FIX-NEXT:aesmc.8 q8, q0
@@ -87,6 +89,7 @@
 ; CHECK-FIX-NEXT:push {r4, lr}
 ; CHECK-FIX-NEXT:mov r4, r0
 ; CHECK-FIX-NEXT:bl get_inputf32
+; CHECK-FIX-NEXT:vorr q0, q0, q0
 ; CHECK-FIX-NEXT:vld1.64 {d16, d17}, [r4]
 ; CHECK-FIX-NEXT:aese.8 q0, q8
 ; CHECK-FIX-NEXT:aesmc.8 q8, q0
@@ -120,6 +123,8 @@
 define arm_aapcs_vfpcc <16 x i8> @aese_once_via_val(<16 x i8> %0, <16 x i8> %1) nounwind {
 ; CHECK-FIX-LABEL: aese_once_via_val:
 ; CHECK-FIX:   @ %bb.0:
+; CHECK-FIX-NEXT:vorr q1, q1, q1
+; CHECK-FIX-NEXT:vorr q0, q0, q0
 ; CHECK-FIX-NEXT:aese.8 q1, q0
 ; CHECK-FIX-NEXT:aesmc.8 q0, q1
 ; CHECK-FIX-NEXT:bx lr
@@ -156,6 +161,9 @@
 define arm_aapcs_vfpcc <16 x i8> @aese_twice_via_val(<16 x i8> %0, <16 x i8> %1) nounwind {
 ; CHECK-FIX-LABEL: aese_twice_via_val:
 ; CHECK-FIX:   @ %bb.0:
+; CHECK-FIX-NEXT:vorr q1, q1, q1
+; CHECK-FIX-NEXT:vorr q0, q0, q0
+; CHECK-FIX-NEXT:vorr q0, q0, q0
 ; CHECK-FIX-NEXT:aese.8 q1, q0
 ; CHECK-FIX-NEXT:aesmc.8 q8, q1
 ; CHECK-FIX-NEXT:aese.8 q8, q0
@@ -219,6 +227,8 @@
 define arm_aapcs_vfpcc <16 x i8> @aese_loop_via_val(i32 %0, <16 x i8> %1, <16 x i8> %2) nounwind {
 ; CHECK-FIX-LABEL: aese_loop_via_val:
 ; CHECK-FIX:   @ %bb.0:
+; CHECK-FIX-NEXT:vorr q1, q1, q1
+; CHECK-FIX-NEXT:vorr q0, q0, q0
 ; CHECK-FIX-NEXT:cmp r0, #0
 ; CHECK-FIX-NEXT:beq .LBB9_2
 ; CHECK-FIX-NEXT:  .LBB9_1: @ =>This Inner Loop Header: Depth=1
@@ -249,6 +259,7 @@
 define arm_aapcs_vfpcc void @aese_set8_via_ptr(i8* %0, <16 x i8> %1, <16 x i8>* %2) nounwind {
 ; CHECK-FIX-NOSCHED-LABEL: aese_set8_via_ptr:
 ; CHECK-FIX-NOSCHED:   @ %bb.0:
+; CHECK-FIX-NOSCHED-NEXT:vorr q0, q0, q0
 ; CHECK-FIX-NOSCHED-NEXT:ldrb r0, [r0]
 ; CHECK-FIX-NOSCHED-NEXT:vld1.64 {d16, d17}, [r1]
 ; CHECK-FIX-NOSCHED-NEXT:vmov.8 d0[0], r0
@@ -260,6 +271,7 @@
 ;
 ; CHECK-CORTEX-FIX-LABEL: aese_set8_via_ptr:
 ; CHECK-CORTEX-FIX:   @ %bb.0:
+; CHECK-CORTEX-FIX-NEXT:vorr q0, q0, q0
 ; CHECK-CORTEX-FIX-NEXT:vld1.64 {d16, d17}, [r1]
 ; CHECK-CORTEX-FIX-NEXT:ldrb r0, [r0]
 ; CHECK-CORTEX-FIX-NEXT:vmov.8 d0[0], r0
@@ -281,6 +293,7 @@
 define arm_aapcs_vfpcc void @aese_set8_via_val(i8 zeroext %0, <16 x i8> %1, <16 x i8>* %2) nounwind {
 ; CHECK-FIX-LABEL: aese_set8_via_val:
 ; CHECK-FIX:   @ %bb.0:
+; CHECK-FIX-NEXT:vorr q0, q0, q0
 ; CHECK-FIX-NEXT:vld1.64 {d16, d17}, [r1]
 ; CHECK-FIX-NEXT:vmov.8 d0[0], r0
 ; CHECK-FIX-NEXT:vmov.8 d16[0], r0
@@ -300,6 +313,7 @@
 define arm_aapcs_vfpcc void @aese_set8_cond_via_ptr(i1 zeroext %0, i8* %1, <16 x i8> %2, <16 x i8>* %3) nounwind {
 ; CHECK-FIX-LABEL: aese_set8_cond_via_ptr:
 ; CHECK-FIX:   @ %bb.0:
+; CHECK-FIX-NEXT:vorr q0, q0, q0
 ; CHECK-FIX-NEXT:cmp r0, #0
 ; CHECK-FIX-NEXT:beq .LBB12_2
 ; CHECK-FIX-NEXT:  @ %bb.1:
@@ -351,6 +365,7 @@
 define arm_aapcs_vfpcc void @aese_set8_cond_via_val(i1 zeroext %0, i8 zeroext %1, <16 x i8> %2, <16 x i8>* %3) nounwind {
 ; CHECK-FIX-LABEL: aese_set8_cond_via_val:
 ; CHECK-FIX:   @ %bb.0:
+; CHECK-FIX-NEXT:vorr q0, q0, q0
 ; CHECK-FIX-NEXT:vld1.64 {d16, d17}, [r2]
 ; CHECK-FIX-NEXT:cmp r0, #0
 ; CHECK-FIX-NEXT:beq .LBB13_2
@@ -380,6 +395,7 @@
 define arm_aapcs_vfpcc void @aese_set8_loop_via_ptr(i32 %0, i8* %1, <16 x i8> %2, <16 x i8>* %3) nounwind {
 ; CHECK-FIX-LABEL: aese_set8_loop_via_ptr:
 ; CHECK-FIX:   @ %bb.0:
+; CHECK-FIX-NEXT:vorr q0, q0, q0
 ; CHECK-FIX-NEXT:ldrb r1, [r1]
 ; CHECK-FIX-NEXT:

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

[Sam Elliott via Phabricator via cfe-commits]

lenary updated this revision to Diff 428978.
lenary marked 3 inline comments as done.
lenary added a comment.

- Address comment nits
- Rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119720

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Arch/ARM.cpp
  clang/test/Driver/arm-fix-cortex-a57-aes-1742098.c
  llvm/lib/Target/ARM/ARM.h
  llvm/lib/Target/ARM/ARM.td
  llvm/lib/Target/ARM/ARMFixCortexA57AES1742098Pass.cpp
  llvm/lib/Target/ARM/ARMTargetMachine.cpp
  llvm/lib/Target/ARM/CMakeLists.txt
  llvm/test/CodeGen/ARM/O3-pipeline.ll
  llvm/test/CodeGen/ARM/aes-erratum-fix.ll

Index: llvm/test/CodeGen/ARM/aes-erratum-fix.ll
===
--- llvm/test/CodeGen/ARM/aes-erratum-fix.ll
+++ llvm/test/CodeGen/ARM/aes-erratum-fix.ll
@@ -47,6 +47,7 @@
 ; CHECK-FIX-NEXT:push {r4, lr}
 ; CHECK-FIX-NEXT:mov r4, r0
 ; CHECK-FIX-NEXT:bl get_input
+; CHECK-FIX-NEXT:vorr q0, q0, q0
 ; CHECK-FIX-NEXT:vld1.64 {d16, d17}, [r4]
 ; CHECK-FIX-NEXT:aese.8 q0, q8
 ; CHECK-FIX-NEXT:aesmc.8 q8, q0
@@ -67,6 +68,7 @@
 ; CHECK-FIX-NEXT:push {r4, lr}
 ; CHECK-FIX-NEXT:mov r4, r0
 ; CHECK-FIX-NEXT:bl get_inputf16
+; CHECK-FIX-NEXT:vorr q0, q0, q0
 ; CHECK-FIX-NEXT:vld1.64 {d16, d17}, [r4]
 ; CHECK-FIX-NEXT:aese.8 q0, q8
 ; CHECK-FIX-NEXT:aesmc.8 q8, q0
@@ -87,6 +89,7 @@
 ; CHECK-FIX-NEXT:push {r4, lr}
 ; CHECK-FIX-NEXT:mov r4, r0
 ; CHECK-FIX-NEXT:bl get_inputf32
+; CHECK-FIX-NEXT:vorr q0, q0, q0
 ; CHECK-FIX-NEXT:vld1.64 {d16, d17}, [r4]
 ; CHECK-FIX-NEXT:aese.8 q0, q8
 ; CHECK-FIX-NEXT:aesmc.8 q8, q0
@@ -120,6 +123,8 @@
 define arm_aapcs_vfpcc <16 x i8> @aese_once_via_val(<16 x i8> %0, <16 x i8> %1) nounwind {
 ; CHECK-FIX-LABEL: aese_once_via_val:
 ; CHECK-FIX:   @ %bb.0:
+; CHECK-FIX-NEXT:vorr q1, q1, q1
+; CHECK-FIX-NEXT:vorr q0, q0, q0
 ; CHECK-FIX-NEXT:aese.8 q1, q0
 ; CHECK-FIX-NEXT:aesmc.8 q0, q1
 ; CHECK-FIX-NEXT:bx lr
@@ -156,6 +161,9 @@
 define arm_aapcs_vfpcc <16 x i8> @aese_twice_via_val(<16 x i8> %0, <16 x i8> %1) nounwind {
 ; CHECK-FIX-LABEL: aese_twice_via_val:
 ; CHECK-FIX:   @ %bb.0:
+; CHECK-FIX-NEXT:vorr q1, q1, q1
+; CHECK-FIX-NEXT:vorr q0, q0, q0
+; CHECK-FIX-NEXT:vorr q0, q0, q0
 ; CHECK-FIX-NEXT:aese.8 q1, q0
 ; CHECK-FIX-NEXT:aesmc.8 q8, q1
 ; CHECK-FIX-NEXT:aese.8 q8, q0
@@ -219,6 +227,8 @@
 define arm_aapcs_vfpcc <16 x i8> @aese_loop_via_val(i32 %0, <16 x i8> %1, <16 x i8> %2) nounwind {
 ; CHECK-FIX-LABEL: aese_loop_via_val:
 ; CHECK-FIX:   @ %bb.0:
+; CHECK-FIX-NEXT:vorr q1, q1, q1
+; CHECK-FIX-NEXT:vorr q0, q0, q0
 ; CHECK-FIX-NEXT:cmp r0, #0
 ; CHECK-FIX-NEXT:beq .LBB9_2
 ; CHECK-FIX-NEXT:  .LBB9_1: @ =>This Inner Loop Header: Depth=1
@@ -249,6 +259,7 @@
 define arm_aapcs_vfpcc void @aese_set8_via_ptr(i8* %0, <16 x i8> %1, <16 x i8>* %2) nounwind {
 ; CHECK-FIX-NOSCHED-LABEL: aese_set8_via_ptr:
 ; CHECK-FIX-NOSCHED:   @ %bb.0:
+; CHECK-FIX-NOSCHED-NEXT:vorr q0, q0, q0
 ; CHECK-FIX-NOSCHED-NEXT:ldrb r0, [r0]
 ; CHECK-FIX-NOSCHED-NEXT:vld1.64 {d16, d17}, [r1]
 ; CHECK-FIX-NOSCHED-NEXT:vmov.8 d0[0], r0
@@ -260,6 +271,7 @@
 ;
 ; CHECK-CORTEX-FIX-LABEL: aese_set8_via_ptr:
 ; CHECK-CORTEX-FIX:   @ %bb.0:
+; CHECK-CORTEX-FIX-NEXT:vorr q0, q0, q0
 ; CHECK-CORTEX-FIX-NEXT:vld1.64 {d16, d17}, [r1]
 ; CHECK-CORTEX-FIX-NEXT:ldrb r0, [r0]
 ; CHECK-CORTEX-FIX-NEXT:vmov.8 d0[0], r0
@@ -281,6 +293,7 @@
 define arm_aapcs_vfpcc void @aese_set8_via_val(i8 zeroext %0, <16 x i8> %1, <16 x i8>* %2) nounwind {
 ; CHECK-FIX-LABEL: aese_set8_via_val:
 ; CHECK-FIX:   @ %bb.0:
+; CHECK-FIX-NEXT:vorr q0, q0, q0
 ; CHECK-FIX-NEXT:vld1.64 {d16, d17}, [r1]
 ; CHECK-FIX-NEXT:vmov.8 d0[0], r0
 ; CHECK-FIX-NEXT:vmov.8 d16[0], r0
@@ -300,6 +313,7 @@
 define arm_aapcs_vfpcc void @aese_set8_cond_via_ptr(i1 zeroext %0, i8* %1, <16 x i8> %2, <16 x i8>* %3) nounwind {
 ; CHECK-FIX-LABEL: aese_set8_cond_via_ptr:
 ; CHECK-FIX:   @ %bb.0:
+; CHECK-FIX-NEXT:vorr q0, q0, q0
 ; CHECK-FIX-NEXT:cmp r0, #0
 ; CHECK-FIX-NEXT:beq .LBB12_2
 ; CHECK-FIX-NEXT:  @ %bb.1:
@@ -351,6 +365,7 @@
 define arm_aapcs_vfpcc void @aese_set8_cond_via_val(i1 zeroext %0, i8 zeroext %1, <16 x i8> %2, <16 x i8>* %3) nounwind {
 ; CHECK-FIX-LABEL: aese_set8_cond_via_val:
 ; CHECK-FIX:   @ %bb.0:
+; CHECK-FIX-NEXT:vorr q0, q0, q0
 ; CHECK-FIX-NEXT:vld1.64 {d16, d17}, [r2]
 ; CHECK-FIX-NEXT:cmp r0, #0
 ; CHECK-FIX-NEXT:beq .LBB13_2
@@ -380,6 +395,7 @@
 define arm_aapcs_vfpcc void @aese_set8_loop_via_ptr(i32 %0, i8* %1, <16 x i8> %2, <16 x i8>* %3) nounwind {
 ; CHECK-FIX-LABEL: aese_set8_loop_via_ptr:
 ; CHECK-FIX:   @ %bb.0:
+; CHECK-FIX-NEXT:vorr q0, q0, q0
 ; CHECK-FIX-NEXT:ldrb r1, [r1]
 ; CHECK-FIX-NEXT:cmp r0, #0
 ; CHECK-FIX-NEXT:s

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

[Sam Elliott via Phabricator via cfe-commits]

lenary marked 3 inline comments as done.
lenary added inline comments.



Comment at: llvm/lib/Target/ARM/ARMFixCortexA57AES1742098Pass.cpp:12-19
+// The intention is this:
+// - Any 128-bit or 64-bit writes to the neon input register of an AES fused
+//   pair are safe (the inputs are to the AESE/AESD instruction).
+// - Any 32-bit writes to the input register are unsafe, but these may happen
+//   in another function, or only on some control flow paths. In these cases,
+//   conservatively insert the VORRq anyway.
+// - So, analyse both inputs to the AESE/AESD instruction, inserting a VORR if

simon_tatham wrote:
> This description would leave me still confused if I didn't happen to already 
> know roughly what the plan was. It jumps in half way through the explanation 
> that someone would need if they were coming to this pass cold. (E.g. it talks 
> about "the VORRq" before having even mentioned //that// there is one, let 
> alone //why// there is one.)
> 
> How about the suggested text as a rewording?
I have taken this feedback on board, and reworded the explanation based on your 
suggestion, with some slight ordering changes and a little more detail about 
the complex cases and some separation between the erratum itself, and the 
workaround we have chosen.


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

[Sam Elliott via Phabricator via cfe-commits]

lenary added a comment.

I want to keep the number of llvm functions the same, as they cover a wide 
variety of control flow at a wide variety of data widths which the pass should 
cope with.

One thing that I would like to cut down is the duplicated set of CHECK lines, 
one with scheduling and one without, but I'm not sure how to do that (the set 
with scheduling are there to test that `-mcpu=cortex-a55` enables the pass, I'm 
not sure of a better way of doing the same.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122747

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

[PATCH] D122747: [NFC][ARM] Tests for Cortex-A57 and Cortex-A72 Fused AES Erratum

[Sam Elliott via Phabricator via cfe-commits]

lenary added a comment.

In D122747#3509495 , @lenary wrote:

> I want to keep the number of llvm functions the same, as they cover a wide 
> variety of control flow at a wide variety of data widths which the pass 
> should cope with.
>
> One thing that I would like to cut down is the duplicated set of CHECK lines, 
> one with scheduling and one without, but I'm not sure how to do that (the set 
> with scheduling are there to test that `-mcpu=cortex-a55` enables the pass), 
> I'm not sure of a better way of doing the same.

After a quick look, I can confirm that the ARM Backend seems to disregard 
`tune-cpu` metadata, the subtarget is initialized with TuneCPU equal to CPU, 
which makes this difficult. Still investigating whether 
`--enable-misched=false` will be enough.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122747

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

[PATCH] D122747: [NFC][ARM] Tests for Cortex-A57 and Cortex-A72 Fused AES Erratum

[Sam Elliott via Phabricator via cfe-commits]

lenary added a comment.

I spent some time last night trying to work out if a combination of 
`--enable-misched=false --pre-RA-sched=linearize -disable-post-ra 
-enable-post-misched=false` would help avoid having two sets of match lines, 
and none seemed to. @MaskRay as I can't find a quick way of making it smaller, 
and the patch these tests are for has been approved, I'm going to land this 
as-is. I hope that is ok.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122747

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

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

[Sam Elliott via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG3a24df992cf8: [ARM] Pass for Cortex-A57 and Cortex-A72 Fused 
AES Erratum (authored by lenary).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119720

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Arch/ARM.cpp
  clang/test/Driver/arm-fix-cortex-a57-aes-1742098.c
  llvm/lib/Target/ARM/ARM.h
  llvm/lib/Target/ARM/ARM.td
  llvm/lib/Target/ARM/ARMFixCortexA57AES1742098Pass.cpp
  llvm/lib/Target/ARM/ARMTargetMachine.cpp
  llvm/lib/Target/ARM/CMakeLists.txt
  llvm/test/CodeGen/ARM/O3-pipeline.ll
  llvm/test/CodeGen/ARM/aes-erratum-fix.ll

Index: llvm/test/CodeGen/ARM/aes-erratum-fix.ll
===
--- llvm/test/CodeGen/ARM/aes-erratum-fix.ll
+++ llvm/test/CodeGen/ARM/aes-erratum-fix.ll
@@ -47,6 +47,7 @@
 ; CHECK-FIX-NEXT:push {r4, lr}
 ; CHECK-FIX-NEXT:mov r4, r0
 ; CHECK-FIX-NEXT:bl get_input
+; CHECK-FIX-NEXT:vorr q0, q0, q0
 ; CHECK-FIX-NEXT:vld1.64 {d16, d17}, [r4]
 ; CHECK-FIX-NEXT:aese.8 q0, q8
 ; CHECK-FIX-NEXT:aesmc.8 q8, q0
@@ -67,6 +68,7 @@
 ; CHECK-FIX-NEXT:push {r4, lr}
 ; CHECK-FIX-NEXT:mov r4, r0
 ; CHECK-FIX-NEXT:bl get_inputf16
+; CHECK-FIX-NEXT:vorr q0, q0, q0
 ; CHECK-FIX-NEXT:vld1.64 {d16, d17}, [r4]
 ; CHECK-FIX-NEXT:aese.8 q0, q8
 ; CHECK-FIX-NEXT:aesmc.8 q8, q0
@@ -87,6 +89,7 @@
 ; CHECK-FIX-NEXT:push {r4, lr}
 ; CHECK-FIX-NEXT:mov r4, r0
 ; CHECK-FIX-NEXT:bl get_inputf32
+; CHECK-FIX-NEXT:vorr q0, q0, q0
 ; CHECK-FIX-NEXT:vld1.64 {d16, d17}, [r4]
 ; CHECK-FIX-NEXT:aese.8 q0, q8
 ; CHECK-FIX-NEXT:aesmc.8 q8, q0
@@ -120,6 +123,8 @@
 define arm_aapcs_vfpcc <16 x i8> @aese_once_via_val(<16 x i8> %0, <16 x i8> %1) nounwind {
 ; CHECK-FIX-LABEL: aese_once_via_val:
 ; CHECK-FIX:   @ %bb.0:
+; CHECK-FIX-NEXT:vorr q1, q1, q1
+; CHECK-FIX-NEXT:vorr q0, q0, q0
 ; CHECK-FIX-NEXT:aese.8 q1, q0
 ; CHECK-FIX-NEXT:aesmc.8 q0, q1
 ; CHECK-FIX-NEXT:bx lr
@@ -156,6 +161,9 @@
 define arm_aapcs_vfpcc <16 x i8> @aese_twice_via_val(<16 x i8> %0, <16 x i8> %1) nounwind {
 ; CHECK-FIX-LABEL: aese_twice_via_val:
 ; CHECK-FIX:   @ %bb.0:
+; CHECK-FIX-NEXT:vorr q1, q1, q1
+; CHECK-FIX-NEXT:vorr q0, q0, q0
+; CHECK-FIX-NEXT:vorr q0, q0, q0
 ; CHECK-FIX-NEXT:aese.8 q1, q0
 ; CHECK-FIX-NEXT:aesmc.8 q8, q1
 ; CHECK-FIX-NEXT:aese.8 q8, q0
@@ -219,6 +227,8 @@
 define arm_aapcs_vfpcc <16 x i8> @aese_loop_via_val(i32 %0, <16 x i8> %1, <16 x i8> %2) nounwind {
 ; CHECK-FIX-LABEL: aese_loop_via_val:
 ; CHECK-FIX:   @ %bb.0:
+; CHECK-FIX-NEXT:vorr q1, q1, q1
+; CHECK-FIX-NEXT:vorr q0, q0, q0
 ; CHECK-FIX-NEXT:cmp r0, #0
 ; CHECK-FIX-NEXT:beq .LBB9_2
 ; CHECK-FIX-NEXT:  .LBB9_1: @ =>This Inner Loop Header: Depth=1
@@ -249,6 +259,7 @@
 define arm_aapcs_vfpcc void @aese_set8_via_ptr(i8* %0, <16 x i8> %1, <16 x i8>* %2) nounwind {
 ; CHECK-FIX-NOSCHED-LABEL: aese_set8_via_ptr:
 ; CHECK-FIX-NOSCHED:   @ %bb.0:
+; CHECK-FIX-NOSCHED-NEXT:vorr q0, q0, q0
 ; CHECK-FIX-NOSCHED-NEXT:ldrb r0, [r0]
 ; CHECK-FIX-NOSCHED-NEXT:vld1.64 {d16, d17}, [r1]
 ; CHECK-FIX-NOSCHED-NEXT:vmov.8 d0[0], r0
@@ -260,6 +271,7 @@
 ;
 ; CHECK-CORTEX-FIX-LABEL: aese_set8_via_ptr:
 ; CHECK-CORTEX-FIX:   @ %bb.0:
+; CHECK-CORTEX-FIX-NEXT:vorr q0, q0, q0
 ; CHECK-CORTEX-FIX-NEXT:vld1.64 {d16, d17}, [r1]
 ; CHECK-CORTEX-FIX-NEXT:ldrb r0, [r0]
 ; CHECK-CORTEX-FIX-NEXT:vmov.8 d0[0], r0
@@ -281,6 +293,7 @@
 define arm_aapcs_vfpcc void @aese_set8_via_val(i8 zeroext %0, <16 x i8> %1, <16 x i8>* %2) nounwind {
 ; CHECK-FIX-LABEL: aese_set8_via_val:
 ; CHECK-FIX:   @ %bb.0:
+; CHECK-FIX-NEXT:vorr q0, q0, q0
 ; CHECK-FIX-NEXT:vld1.64 {d16, d17}, [r1]
 ; CHECK-FIX-NEXT:vmov.8 d0[0], r0
 ; CHECK-FIX-NEXT:vmov.8 d16[0], r0
@@ -300,6 +313,7 @@
 define arm_aapcs_vfpcc void @aese_set8_cond_via_ptr(i1 zeroext %0, i8* %1, <16 x i8> %2, <16 x i8>* %3) nounwind {
 ; CHECK-FIX-LABEL: aese_set8_cond_via_ptr:
 ; CHECK-FIX:   @ %bb.0:
+; CHECK-FIX-NEXT:vorr q0, q0, q0
 ; CHECK-FIX-NEXT:cmp r0, #0
 ; CHECK-FIX-NEXT:beq .LBB12_2
 ; CHECK-FIX-NEXT:  @ %bb.1:
@@ -351,6 +365,7 @@
 define arm_aapcs_vfpcc void @aese_set8_cond_via_val(i1 zeroext %0, i8 zeroext %1, <16 x i8> %2, <16 x i8>* %3) nounwind {
 ; CHECK-FIX-LABEL: aese_set8_cond_via_val:
 ; CHECK-FIX:   @ %bb.0:
+; CHECK-FIX-NEXT:vorr q0, q0, q0
 ; CHECK-FIX-NEXT:vld1.64 {d16, d17}, [r2]
 ; CHECK-FIX-NEXT:cmp r0, #0
 ; CHECK-FIX-NEXT:beq .LBB13_2
@@ -380,6 +395,7 @@
 define arm_aapcs_vfpcc void @aese_set8_loop_via_ptr(i32 %0, i8* %1, <16 x i8> %2, <16 x i8>* %3) nounwind {
 ; CHECK-FIX-LABEL: aese_set8_loop_via_ptr:
 ; CHECK-FIX:   @ %bb.0:
+; CHECK-FIX-NEXT:vorr q0

[PATCH] D125775: [ARM] Don't Enable AES Pass for Generic Cores

[Sam Elliott via Phabricator via cfe-commits]

lenary created this revision.
lenary added reviewers: john.brawn, dmgreen.
Herald added subscribers: hiraditya, kristof.beyls.
Herald added a project: All.
lenary requested review of this revision.
Herald added projects: clang, LLVM.
Herald added a subscriber: cfe-commits.

This brings clang/llvm into line with GCC. The Pass is still enabled for
the affected cores, but is now opt-in when using `-march=`.

I also took the opportunity to add release notes for this change.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D125775

Files:
  clang/docs/ReleaseNotes.rst
  llvm/docs/ReleaseNotes.rst
  llvm/lib/Target/ARM/ARM.td


Index: llvm/lib/Target/ARM/ARM.td
===
--- llvm/lib/Target/ARM/ARM.td
+++ llvm/lib/Target/ARM/ARM.td
@@ -1161,7 +1161,7 @@
 // ARM processors
 //
 // Dummy CPU, used to target architectures
-def : ProcessorModel<"generic", CortexA8Model,  
[FeatureFixCortexA57AES1742098]>;
+def : ProcessorModel<"generic", CortexA8Model,  []>;
 
 // FIXME: Several processors below are not using their own scheduler
 // model, but one of similar/previous processor. These should be fixed.
Index: llvm/docs/ReleaseNotes.rst
===
--- llvm/docs/ReleaseNotes.rst
+++ llvm/docs/ReleaseNotes.rst
@@ -99,6 +99,8 @@
   warnings will be generated and -mrestrict-it is now always off by default.
   Previously it was on by default for Armv8 and off for all other architecture
   versions.
+* Added a pass to workaround Cortex-A57 Erratum 1742098 and Cortex-A72
+  Erratum 1655431. This is enabled by default when targeting either CPU.
 
 Changes to the AVR Backend
 --
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -265,6 +265,11 @@
   the parameter list were ``void``. There is no ``-fknr-functions`` or
   ``-fno-no-knr-functions`` flag; this feature cannot be disabled in language
   modes where it is required, such as C++ or C2x.
+- A new ARM pass to workaround Cortex-A57 Erratum 1742098 and Cortex-A72 
Erratum
+  1655431 can be enabled using ``-mfix-cortex-a57-aes-1742098`` or
+  ``-mfix-cortex-a72-aes-1655431``. The pass is enabled when using either of
+  these cpus with ``-mcpu=`` and can be disabled using
+  ``-mno-fix-cortex-a57-aes-1742098`` or ``-mno-fix-cortex-a72-aes-1655431``.
 
 Deprecated Compiler Flags
 -


Index: llvm/lib/Target/ARM/ARM.td
===
--- llvm/lib/Target/ARM/ARM.td
+++ llvm/lib/Target/ARM/ARM.td
@@ -1161,7 +1161,7 @@
 // ARM processors
 //
 // Dummy CPU, used to target architectures
-def : ProcessorModel<"generic", CortexA8Model,  [FeatureFixCortexA57AES1742098]>;
+def : ProcessorModel<"generic", CortexA8Model,  []>;
 
 // FIXME: Several processors below are not using their own scheduler
 // model, but one of similar/previous processor. These should be fixed.
Index: llvm/docs/ReleaseNotes.rst
===
--- llvm/docs/ReleaseNotes.rst
+++ llvm/docs/ReleaseNotes.rst
@@ -99,6 +99,8 @@
   warnings will be generated and -mrestrict-it is now always off by default.
   Previously it was on by default for Armv8 and off for all other architecture
   versions.
+* Added a pass to workaround Cortex-A57 Erratum 1742098 and Cortex-A72
+  Erratum 1655431. This is enabled by default when targeting either CPU.
 
 Changes to the AVR Backend
 --
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -265,6 +265,11 @@
   the parameter list were ``void``. There is no ``-fknr-functions`` or
   ``-fno-no-knr-functions`` flag; this feature cannot be disabled in language
   modes where it is required, such as C++ or C2x.
+- A new ARM pass to workaround Cortex-A57 Erratum 1742098 and Cortex-A72 Erratum
+  1655431 can be enabled using ``-mfix-cortex-a57-aes-1742098`` or
+  ``-mfix-cortex-a72-aes-1655431``. The pass is enabled when using either of
+  these cpus with ``-mcpu=`` and can be disabled using
+  ``-mno-fix-cortex-a57-aes-1742098`` or ``-mno-fix-cortex-a72-aes-1655431``.
 
 Deprecated Compiler Flags
 -
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D125775: [ARM] Don't Enable AES Pass for Generic Cores

[Sam Elliott 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 rG2321c36fbf76: [ARM] Don't Enable AES Pass for Generic 
Cores (authored by lenary).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125775

Files:
  clang/docs/ReleaseNotes.rst
  llvm/docs/ReleaseNotes.rst
  llvm/lib/Target/ARM/ARM.td


Index: llvm/lib/Target/ARM/ARM.td
===
--- llvm/lib/Target/ARM/ARM.td
+++ llvm/lib/Target/ARM/ARM.td
@@ -1161,7 +1161,7 @@
 // ARM processors
 //
 // Dummy CPU, used to target architectures
-def : ProcessorModel<"generic", CortexA8Model,  
[FeatureFixCortexA57AES1742098]>;
+def : ProcessorModel<"generic", CortexA8Model,  []>;
 
 // FIXME: Several processors below are not using their own scheduler
 // model, but one of similar/previous processor. These should be fixed.
Index: llvm/docs/ReleaseNotes.rst
===
--- llvm/docs/ReleaseNotes.rst
+++ llvm/docs/ReleaseNotes.rst
@@ -99,6 +99,8 @@
   warnings will be generated and -mrestrict-it is now always off by default.
   Previously it was on by default for Armv8 and off for all other architecture
   versions.
+* Added a pass to workaround Cortex-A57 Erratum 1742098 and Cortex-A72
+  Erratum 1655431. This is enabled by default when targeting either CPU.
 
 Changes to the AVR Backend
 --
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -265,6 +265,11 @@
   the parameter list were ``void``. There is no ``-fknr-functions`` or
   ``-fno-no-knr-functions`` flag; this feature cannot be disabled in language
   modes where it is required, such as C++ or C2x.
+- A new ARM pass to workaround Cortex-A57 Erratum 1742098 and Cortex-A72 
Erratum
+  1655431 can be enabled using ``-mfix-cortex-a57-aes-1742098`` or
+  ``-mfix-cortex-a72-aes-1655431``. The pass is enabled when using either of
+  these cpus with ``-mcpu=`` and can be disabled using
+  ``-mno-fix-cortex-a57-aes-1742098`` or ``-mno-fix-cortex-a72-aes-1655431``.
 
 Deprecated Compiler Flags
 -


Index: llvm/lib/Target/ARM/ARM.td
===
--- llvm/lib/Target/ARM/ARM.td
+++ llvm/lib/Target/ARM/ARM.td
@@ -1161,7 +1161,7 @@
 // ARM processors
 //
 // Dummy CPU, used to target architectures
-def : ProcessorModel<"generic", CortexA8Model,  [FeatureFixCortexA57AES1742098]>;
+def : ProcessorModel<"generic", CortexA8Model,  []>;
 
 // FIXME: Several processors below are not using their own scheduler
 // model, but one of similar/previous processor. These should be fixed.
Index: llvm/docs/ReleaseNotes.rst
===
--- llvm/docs/ReleaseNotes.rst
+++ llvm/docs/ReleaseNotes.rst
@@ -99,6 +99,8 @@
   warnings will be generated and -mrestrict-it is now always off by default.
   Previously it was on by default for Armv8 and off for all other architecture
   versions.
+* Added a pass to workaround Cortex-A57 Erratum 1742098 and Cortex-A72
+  Erratum 1655431. This is enabled by default when targeting either CPU.
 
 Changes to the AVR Backend
 --
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -265,6 +265,11 @@
   the parameter list were ``void``. There is no ``-fknr-functions`` or
   ``-fno-no-knr-functions`` flag; this feature cannot be disabled in language
   modes where it is required, such as C++ or C2x.
+- A new ARM pass to workaround Cortex-A57 Erratum 1742098 and Cortex-A72 Erratum
+  1655431 can be enabled using ``-mfix-cortex-a57-aes-1742098`` or
+  ``-mfix-cortex-a72-aes-1655431``. The pass is enabled when using either of
+  these cpus with ``-mcpu=`` and can be disabled using
+  ``-mno-fix-cortex-a57-aes-1742098`` or ``-mno-fix-cortex-a72-aes-1655431``.
 
 Deprecated Compiler Flags
 -
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Sam Elliott via Phabricator via cfe-commits]

lenary updated this revision to Diff 216594.
lenary added a comment.
Herald added a subscriber: pzheng.

Update MaxAtomicPromote width to treat it like an ABI feature, and set it to 128


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D57450

Files:
  clang/lib/Basic/Targets/RISCV.h
  clang/test/CodeGen/riscv-atomics.c

Index: clang/test/CodeGen/riscv-atomics.c
===
--- /dev/null
+++ clang/test/CodeGen/riscv-atomics.c
@@ -0,0 +1,68 @@
+// RUN: %clang_cc1 -triple riscv32 -O1 -emit-llvm %s -o - \
+// RUN:   | FileCheck %s -check-prefix=RV32I
+// RUN: %clang_cc1 -triple riscv32 -target-feature +a -O1 -emit-llvm %s -o - \
+// RUN:   | FileCheck %s -check-prefix=RV32IA
+// RUN: %clang_cc1 -triple riscv64 -O1 -emit-llvm %s -o - \
+// RUN:   | FileCheck %s -check-prefix=RV64I
+// RUN: %clang_cc1 -triple riscv64 -target-feature +a -O1 -emit-llvm %s -o - \
+// RUN:   | FileCheck %s -check-prefix=RV64IA
+
+// This test demonstrates that MaxAtomicInlineWidth is set appropriately when
+// the atomics instruction set extension is enabled.
+
+#include 
+#include 
+
+void test_i8_atomics(_Atomic(int8_t) * a, int8_t b) {
+  // RV32I:  call zeroext i8 @__atomic_load_1
+  // RV32I:  call void @__atomic_store_1
+  // RV32I:  call zeroext i8 @__atomic_fetch_add_1
+  // RV32IA: load atomic i8, i8* %a seq_cst, align 1
+  // RV32IA: store atomic i8 %b, i8* %a seq_cst, align 1
+  // RV32IA: atomicrmw add i8* %a, i8 %b seq_cst
+  // RV64I:  call zeroext i8 @__atomic_load_1
+  // RV64I:  call void @__atomic_store_1
+  // RV64I:  call zeroext i8 @__atomic_fetch_add_1
+  // RV64IA: load atomic i8, i8* %a seq_cst, align 1
+  // RV64IA: store atomic i8 %b, i8* %a seq_cst, align 1
+  // RV64IA: atomicrmw add i8* %a, i8 %b seq_cst
+  __c11_atomic_load(a, memory_order_seq_cst);
+  __c11_atomic_store(a, b, memory_order_seq_cst);
+  __c11_atomic_fetch_add(a, b, memory_order_seq_cst);
+}
+
+void test_i32_atomics(_Atomic(int32_t) * a, int32_t b) {
+  // RV32I:  call i32 @__atomic_load_4
+  // RV32I:  call void @__atomic_store_4
+  // RV32I:  call i32 @__atomic_fetch_add_4
+  // RV32IA: load atomic i32, i32* %a seq_cst, align 4
+  // RV32IA: store atomic i32 %b, i32* %a seq_cst, align 4
+  // RV32IA: atomicrmw add i32* %a, i32 %b seq_cst
+  // RV64I:  call signext i32 @__atomic_load_4
+  // RV64I:  call void @__atomic_store_4
+  // RV64I:  call signext i32 @__atomic_fetch_add_4
+  // RV64IA: load atomic i32, i32* %a seq_cst, align 4
+  // RV64IA: store atomic i32 %b, i32* %a seq_cst, align 4
+  // RV64IA: atomicrmw add i32* %a, i32 %b seq_cst
+  __c11_atomic_load(a, memory_order_seq_cst);
+  __c11_atomic_store(a, b, memory_order_seq_cst);
+  __c11_atomic_fetch_add(a, b, memory_order_seq_cst);
+}
+
+void test_i64_atomics(_Atomic(int64_t) * a, int64_t b) {
+  // RV32I:  call i64 @__atomic_load_8
+  // RV32I:  call void @__atomic_store_8
+  // RV32I:  call i64 @__atomic_fetch_add_8
+  // RV32IA: call i64 @__atomic_load_8
+  // RV32IA: call void @__atomic_store_8
+  // RV32IA: call i64 @__atomic_fetch_add_8
+  // RV64I:  call i64 @__atomic_load_8
+  // RV64I:  call void @__atomic_store_8
+  // RV64I:  call i64 @__atomic_fetch_add_8
+  // RV64IA: load atomic i64, i64* %a seq_cst, align 8
+  // RV64IA: store atomic i64 %b, i64* %a seq_cst, align 8
+  // RV64IA: atomicrmw add i64* %a, i64 %b seq_cst
+  __c11_atomic_load(a, memory_order_seq_cst);
+  __c11_atomic_store(a, b, memory_order_seq_cst);
+  __c11_atomic_fetch_add(a, b, memory_order_seq_cst);
+}
Index: clang/lib/Basic/Targets/RISCV.h
===
--- clang/lib/Basic/Targets/RISCV.h
+++ clang/lib/Basic/Targets/RISCV.h
@@ -93,6 +93,13 @@
 }
 return false;
   }
+
+  void setMaxAtomicWidth() override {
+MaxAtomicPromoteWidth = 128;
+
+if (HasA)
+  MaxAtomicInlineWidth = 32;
+  }
 };
 class LLVM_LIBRARY_VISIBILITY RISCV64TargetInfo : public RISCVTargetInfo {
 public:
@@ -110,6 +117,13 @@
 }
 return false;
   }
+
+  void setMaxAtomicWidth() override {
+MaxAtomicPromoteWidth = 128;
+
+if (HasA)
+  MaxAtomicInlineWidth = 64;
+  }
 };
 } // namespace targets
 } // namespace clang
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Sam Elliott via Phabricator via cfe-commits]

lenary added a comment.

Cross linking to the relevant psABI pull request (still pending): 
https://github.com/riscv/riscv-elf-psabi-doc/pull/112


Repository:
  rG LLVM Github Monorepo

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] D66591: [RISCV] Correct Logic around ilp32e macros

[Sam Elliott via Phabricator via cfe-commits]

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

GCC seperates the `__riscv_float_abi_*` macros and the
`__riscv_abi_rve` macro. If the chosen abi is ilp32e, `gcc -march=rv32i
-mabi=ilp32i -E -dM` shows that both `__riscv_float_abi_soft` and
`__riscv_abi_rve` are set.

This patch corrects the compiler logic around these defines.

At the moment, this patch will not change clang's behaviour, because we do not
accept the `ilp32e` abi yet.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D66591

Files:
  clang/lib/Basic/Targets/RISCV.cpp


Index: clang/lib/Basic/Targets/RISCV.cpp
===
--- clang/lib/Basic/Targets/RISCV.cpp
+++ clang/lib/Basic/Targets/RISCV.cpp
@@ -96,11 +96,12 @@
 Builder.defineMacro("__riscv_float_abi_single");
   else if (ABIName == "ilp32d" || ABIName == "lp64d")
 Builder.defineMacro("__riscv_float_abi_double");
-  else if (ABIName == "ilp32e")
-Builder.defineMacro("__riscv_abi_rve");
   else
 Builder.defineMacro("__riscv_float_abi_soft");
 
+  if (ABIName == "ilp32e")
+Builder.defineMacro("__riscv_abi_rve");
+
   if (HasM) {
 Builder.defineMacro("__riscv_mul");
 Builder.defineMacro("__riscv_div");


Index: clang/lib/Basic/Targets/RISCV.cpp
===
--- clang/lib/Basic/Targets/RISCV.cpp
+++ clang/lib/Basic/Targets/RISCV.cpp
@@ -96,11 +96,12 @@
 Builder.defineMacro("__riscv_float_abi_single");
   else if (ABIName == "ilp32d" || ABIName == "lp64d")
 Builder.defineMacro("__riscv_float_abi_double");
-  else if (ABIName == "ilp32e")
-Builder.defineMacro("__riscv_abi_rve");
   else
 Builder.defineMacro("__riscv_float_abi_soft");
 
+  if (ABIName == "ilp32e")
+Builder.defineMacro("__riscv_abi_rve");
+
   if (HasM) {
 Builder.defineMacro("__riscv_mul");
 Builder.defineMacro("__riscv_div");
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Sam Elliott via Phabricator via cfe-commits]

lenary added a comment.

@jyknight I hear where you're coming from. I'll see what I can do about the 
psABI document.

In that ticket, it's mentioned that the Darwin ABI explicitly says that 
non-power-of-two atomic types should be padded and realigned, but I cannot find 
any documentation explaining this. That would be useful, given presumably GCC 
does have to pad/align on Darwin.

Then the only outstanding question relates to zero-sized atomics, which GCC 
does not pad, but I think Clang has to pad to get the semantics correct, based 
on this comment: 
https://github.com/llvm/llvm-project/blob/master/clang/lib/AST/ASTContext.cpp#L2176


Repository:
  rG LLVM Github Monorepo

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] D57450: [RISCV] Set MaxAtomicInlineWidth and MaxAtomicPromoteWidth for RV32/RV64 targets with atomics

[Sam Elliott via Phabricator via cfe-commits]

lenary added a comment.

In D57450#1641308 , @jyknight wrote:

> In D57450#1641190 , @lenary wrote:
>
> > @jyknight I hear where you're coming from. I'll see what I can do about the 
> > psABI document.
> >
> > In that ticket, it's mentioned that the Darwin ABI explicitly says that 
> > non-power-of-two atomic types should be padded and realigned, but I cannot 
> > find any documentation explaining this. That would be useful, given 
> > presumably GCC does have to pad/align on Darwin.
>
>
> AFAIK, there is no such documentation, at least publicly. Possibly Apple has 
> some internally, but I suspect it more likely just some in-person 
> conversation or something.
>
> GCC is not really supported on Darwin, so I suspect it just gets it wrong.


To keep everyone updated, this has bubbled over into a thread on the GCC 
Mailing list: https://gcc.gnu.org/ml/gcc/2019-08/msg00191.html - Potentially 
this is a route to a resolution on non-RISC-V platforms (specifically x86).

> 
> 
>> Then the only outstanding question relates to zero-sized atomics, which GCC 
>> does not pad, but I think Clang has to pad to get the semantics correct, 
>> based on this comment: 
>> https://github.com/llvm/llvm-project/blob/master/clang/lib/AST/ASTContext.cpp#L2176
> 
> The semantics in GCC are that you can create such an object, but any attempt 
> to load or store it will result in a compile-time error. E.g., "error: 
> argument 1 of ‘__atomic_load’ must be a pointer to a nonzero size object". So 
> I don't think there's really an issue there.

Neat, ok.

I think the feeling with this ticket and the RISC-V backend is:

- We match GCC for power-of-two-sized atomics
- We don't match for non-power-of-two sized atomics (as on any other platform)

This feels like that should be enough for this patch to be merged, and in a 
future patch we can address the fall-out of ABIs changing across GCC and clang 
on more platforms than just RISC-V?


Repository:
  rG LLVM Github Monorepo

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] D57450: [RISCV] Set MaxAtomicInlineWidth and MaxAtomicPromoteWidth for RV32/RV64 targets with atomics

[Sam Elliott via Phabricator via cfe-commits]

lenary updated this revision to Diff 217413.
lenary added a comment.

Address review feedback:

- Add Test for MaxAtomicPromoteWidth


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D57450

Files:
  clang/lib/Basic/Targets/RISCV.h
  clang/test/CodeGen/riscv-atomics.c
  clang/test/Driver/riscv32-toolchain.c
  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
@@ -105,6 +105,10 @@
 typedef __SIZE_TYPE__ size_t;
 typedef __PTRDIFF_TYPE__ ptrdiff_t;
 typedef __WCHAR_TYPE__ wchar_t;
+typedef __WINT_TYPE__ wint_t;
+
+
+// Check Alignments
 
 // CHECK: @align_c = dso_local global i32 1
 int align_c = __alignof(char);
@@ -118,6 +122,9 @@
 // CHECK: @align_wc = dso_local global i32 4
 int align_wc = __alignof(wchar_t);
 
+// CHECK: @align_wi = dso_local global i32 4
+int align_wi = __alignof(wint_t);
+
 // CHECK: @align_l = dso_local global i32 8
 int align_l = __alignof(long);
 
@@ -139,6 +146,88 @@
 // CHECK: @align_vl = dso_local global i32 8
 int align_vl = __alignof(va_list);
 
+// CHECK: @align_a_c = dso_local global i32 1
+int align_a_c = __alignof(_Atomic(char));
+
+// CHECK: @align_a_s = dso_local global i32 2
+int align_a_s = __alignof(_Atomic(short));
+
+// CHECK: @align_a_i = dso_local global i32 4
+int align_a_i = __alignof(_Atomic(int));
+
+// CHECK: @align_a_wc = dso_local global i32 4
+int align_a_wc = __alignof(_Atomic(wchar_t));
+
+// CHECK: @align_a_wi = dso_local global i32 4
+int align_a_wi = __alignof(_Atomic(wint_t));
+
+// CHECK: @align_a_l = dso_local global i32 8
+int align_a_l = __alignof(_Atomic(long));
+
+// CHECK: @align_a_ll = dso_local global i32 8
+int align_a_ll = __alignof(_Atomic(long long));
+
+// CHECK: @align_a_p = dso_local global i32 8
+int align_a_p = __alignof(_Atomic(void*));
+
+// CHECK @align_a_f = dso_local global i32 4
+int align_a_f = __alignof(_Atomic(float));
+
+// CHECK: @align_a_d = dso_local global i32 8
+int align_a_d = __alignof(_Atomic(double));
+
+// CHECK: @align_a_ld = dso_local global i32 16
+int align_a_ld = __alignof(_Atomic(long double));
+
+// CHECK: @align_a_s4 = dso_local global i32 4
+int align_a_s4 = __alignof(_Atomic(struct { char _[4]; }));
+
+// CHECK: @align_a_s8 = dso_local global i32 8
+int align_a_s8 = __alignof(_Atomic(struct { char _[8]; }));
+
+// CHECK: @align_a_s16 = dso_local global i32 16
+int align_a_s16 = __alignof(_Atomic(struct { char _[16]; }));
+
+// CHECK: @align_a_s32 = dso_local global i32 1
+int align_a_s32 = __alignof(_Atomic(struct { char _[32]; }));
+
+
+// Check Sizes
+
+// CHECK: @size_a_c = dso_local global i32 1
+int size_a_c = sizeof(_Atomic(char));
+
+// CHECK: @size_a_s = dso_local global i32 2
+int size_a_s = sizeof(_Atomic(short));
+
+// CHECK: @size_a_i = dso_local global i32 4
+int size_a_i = sizeof(_Atomic(int));
+
+// CHECK: @size_a_wc = dso_local global i32 4
+int size_a_wc = sizeof(_Atomic(wchar_t));
+
+// CHECK: @size_a_wi = dso_local global i32 4
+int size_a_wi = sizeof(_Atomic(wint_t));
+
+// CHECK: @size_a_l = dso_local global i32 8
+int size_a_l = sizeof(_Atomic(long));
+
+// CHECK: @size_a_ll = dso_local global i32 8
+int size_a_ll = sizeof(_Atomic(long long));
+
+// CHECK: @size_a_p = dso_local global i32 8
+int size_a_p = sizeof(_Atomic(void*));
+
+// CHECK: @size_a_f = dso_local global i32 4
+int size_a_f = sizeof(_Atomic(float));
+
+// CHECK: @size_a_d = dso_local global i32 8
+int size_a_d = sizeof(_Atomic(double));
+
+// CHECK: @size_a_ld = dso_local global i32 16
+int size_a_ld = sizeof(_Atomic(long double));
+
+
 // Check types
 
 // CHECK: define dso_local zeroext i8 @check_char()
Index: clang/test/Driver/riscv32-toolchain.c
===
--- clang/test/Driver/riscv32-toolchain.c
+++ clang/test/Driver/riscv32-toolchain.c
@@ -105,6 +105,10 @@
 typedef __SIZE_TYPE__ size_t;
 typedef __PTRDIFF_TYPE__ ptrdiff_t;
 typedef __WCHAR_TYPE__ wchar_t;
+typedef __WINT_TYPE__ wint_t;
+
+
+// Check Alignments
 
 // CHECK: @align_c = dso_local global i32 1
 int align_c = __alignof(char);
@@ -118,6 +122,9 @@
 // CHECK: @align_wc = dso_local global i32 4
 int align_wc = __alignof(wchar_t);
 
+// CHECK: @align_wi = dso_local global i32 4
+int align_wi = __alignof(wint_t);
+
 // CHECK: @align_l = dso_local global i32 4
 int align_l = __alignof(long);
 
@@ -139,6 +146,88 @@
 // CHECK: @align_vl = dso_local global i32 4
 int align_vl = __alignof(va_list);
 
+// CHECK: @align_a_c = dso_local global i32 1
+int align_a_c = __alignof(_Atomic(char));
+
+// CHECK: @align_a_s = dso_local global i32 2
+int align_a_s = __alignof(_Atomic(short));
+
+// CHECK: @align_a_i = dso_local global i32 4
+int align_a_i = __alignof(_Atomic(int));
+
+// CHECK: @align_a_wc = dso_local global i32 4
+int align_a_wc = __a

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

[Sam Elliott via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL370073: [RISCV] Set MaxAtomicInlineWidth and 
MaxAtomicPromoteWidth for RV32/RV64… (authored by lenary, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D57450?vs=217413&id=217418#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D57450

Files:
  cfe/trunk/lib/Basic/Targets/RISCV.h
  cfe/trunk/test/CodeGen/riscv-atomics.c
  cfe/trunk/test/Driver/riscv32-toolchain.c
  cfe/trunk/test/Driver/riscv64-toolchain.c

Index: cfe/trunk/lib/Basic/Targets/RISCV.h
===
--- cfe/trunk/lib/Basic/Targets/RISCV.h
+++ cfe/trunk/lib/Basic/Targets/RISCV.h
@@ -93,6 +93,13 @@
 }
 return false;
   }
+
+  void setMaxAtomicWidth() override {
+MaxAtomicPromoteWidth = 128;
+
+if (HasA)
+  MaxAtomicInlineWidth = 32;
+  }
 };
 class LLVM_LIBRARY_VISIBILITY RISCV64TargetInfo : public RISCVTargetInfo {
 public:
@@ -110,6 +117,13 @@
 }
 return false;
   }
+
+  void setMaxAtomicWidth() override {
+MaxAtomicPromoteWidth = 128;
+
+if (HasA)
+  MaxAtomicInlineWidth = 64;
+  }
 };
 } // namespace targets
 } // namespace clang
Index: cfe/trunk/test/CodeGen/riscv-atomics.c
===
--- cfe/trunk/test/CodeGen/riscv-atomics.c
+++ cfe/trunk/test/CodeGen/riscv-atomics.c
@@ -0,0 +1,68 @@
+// RUN: %clang_cc1 -triple riscv32 -O1 -emit-llvm %s -o - \
+// RUN:   | FileCheck %s -check-prefix=RV32I
+// RUN: %clang_cc1 -triple riscv32 -target-feature +a -O1 -emit-llvm %s -o - \
+// RUN:   | FileCheck %s -check-prefix=RV32IA
+// RUN: %clang_cc1 -triple riscv64 -O1 -emit-llvm %s -o - \
+// RUN:   | FileCheck %s -check-prefix=RV64I
+// RUN: %clang_cc1 -triple riscv64 -target-feature +a -O1 -emit-llvm %s -o - \
+// RUN:   | FileCheck %s -check-prefix=RV64IA
+
+// This test demonstrates that MaxAtomicInlineWidth is set appropriately when
+// the atomics instruction set extension is enabled.
+
+#include 
+#include 
+
+void test_i8_atomics(_Atomic(int8_t) * a, int8_t b) {
+  // RV32I:  call zeroext i8 @__atomic_load_1
+  // RV32I:  call void @__atomic_store_1
+  // RV32I:  call zeroext i8 @__atomic_fetch_add_1
+  // RV32IA: load atomic i8, i8* %a seq_cst, align 1
+  // RV32IA: store atomic i8 %b, i8* %a seq_cst, align 1
+  // RV32IA: atomicrmw add i8* %a, i8 %b seq_cst
+  // RV64I:  call zeroext i8 @__atomic_load_1
+  // RV64I:  call void @__atomic_store_1
+  // RV64I:  call zeroext i8 @__atomic_fetch_add_1
+  // RV64IA: load atomic i8, i8* %a seq_cst, align 1
+  // RV64IA: store atomic i8 %b, i8* %a seq_cst, align 1
+  // RV64IA: atomicrmw add i8* %a, i8 %b seq_cst
+  __c11_atomic_load(a, memory_order_seq_cst);
+  __c11_atomic_store(a, b, memory_order_seq_cst);
+  __c11_atomic_fetch_add(a, b, memory_order_seq_cst);
+}
+
+void test_i32_atomics(_Atomic(int32_t) * a, int32_t b) {
+  // RV32I:  call i32 @__atomic_load_4
+  // RV32I:  call void @__atomic_store_4
+  // RV32I:  call i32 @__atomic_fetch_add_4
+  // RV32IA: load atomic i32, i32* %a seq_cst, align 4
+  // RV32IA: store atomic i32 %b, i32* %a seq_cst, align 4
+  // RV32IA: atomicrmw add i32* %a, i32 %b seq_cst
+  // RV64I:  call signext i32 @__atomic_load_4
+  // RV64I:  call void @__atomic_store_4
+  // RV64I:  call signext i32 @__atomic_fetch_add_4
+  // RV64IA: load atomic i32, i32* %a seq_cst, align 4
+  // RV64IA: store atomic i32 %b, i32* %a seq_cst, align 4
+  // RV64IA: atomicrmw add i32* %a, i32 %b seq_cst
+  __c11_atomic_load(a, memory_order_seq_cst);
+  __c11_atomic_store(a, b, memory_order_seq_cst);
+  __c11_atomic_fetch_add(a, b, memory_order_seq_cst);
+}
+
+void test_i64_atomics(_Atomic(int64_t) * a, int64_t b) {
+  // RV32I:  call i64 @__atomic_load_8
+  // RV32I:  call void @__atomic_store_8
+  // RV32I:  call i64 @__atomic_fetch_add_8
+  // RV32IA: call i64 @__atomic_load_8
+  // RV32IA: call void @__atomic_store_8
+  // RV32IA: call i64 @__atomic_fetch_add_8
+  // RV64I:  call i64 @__atomic_load_8
+  // RV64I:  call void @__atomic_store_8
+  // RV64I:  call i64 @__atomic_fetch_add_8
+  // RV64IA: load atomic i64, i64* %a seq_cst, align 8
+  // RV64IA: store atomic i64 %b, i64* %a seq_cst, align 8
+  // RV64IA: atomicrmw add i64* %a, i64 %b seq_cst
+  __c11_atomic_load(a, memory_order_seq_cst);
+  __c11_atomic_store(a, b, memory_order_seq_cst);
+  __c11_atomic_fetch_add(a, b, memory_order_seq_cst);
+}
Index: cfe/trunk/test/Driver/riscv32-toolchain.c
===
--- cfe/trunk/test/Driver/riscv32-toolchain.c
+++ cfe/trunk/test/Driver/riscv32-toolchain.c
@@ -105,6 +105,10 @@
 typedef __SIZE_TYPE__ size_t;
 typedef __PTRDIFF_TYPE__ ptrdiff_t;
 typedef __WCHAR_TYPE__ wchar_t;
+typedef __WINT_TYPE__ wint_t;
+
+
+// Check Alignments
 

[PATCH] D67065: [RISCV] Define __riscv_cmodel_medlow and __riscv_cmodel_medany correctly

[Sam Elliott via Phabricator via cfe-commits]

lenary added inline comments.



Comment at: clang/lib/Basic/Targets/RISCV.cpp:91
   Builder.defineMacro("__riscv_xlen", Is64Bit ? "64" : "32");
   // TODO: modify when more code models are supported.
+  StringRef CodeModel = getTargetOpts().CodeModel;

This TODO can probably be deleted - now it's more obvious where to add more 
code when we support more code models.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67065



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

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

[Sam Elliott via Phabricator via cfe-commits]

lenary added a comment.

Backported to 9.0 in rL370181 


Repository:
  rL LLVM

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] D66591: [RISCV] Correct Logic around ilp32e macros

[Sam Elliott via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL370709: [RISCV] Correct Logic around ilp32e macros (authored 
by lenary, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D66591?vs=216612&id=218409#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D66591

Files:
  cfe/trunk/lib/Basic/Targets/RISCV.cpp


Index: cfe/trunk/lib/Basic/Targets/RISCV.cpp
===
--- cfe/trunk/lib/Basic/Targets/RISCV.cpp
+++ cfe/trunk/lib/Basic/Targets/RISCV.cpp
@@ -96,11 +96,12 @@
 Builder.defineMacro("__riscv_float_abi_single");
   else if (ABIName == "ilp32d" || ABIName == "lp64d")
 Builder.defineMacro("__riscv_float_abi_double");
-  else if (ABIName == "ilp32e")
-Builder.defineMacro("__riscv_abi_rve");
   else
 Builder.defineMacro("__riscv_float_abi_soft");
 
+  if (ABIName == "ilp32e")
+Builder.defineMacro("__riscv_abi_rve");
+
   if (HasM) {
 Builder.defineMacro("__riscv_mul");
 Builder.defineMacro("__riscv_div");


Index: cfe/trunk/lib/Basic/Targets/RISCV.cpp
===
--- cfe/trunk/lib/Basic/Targets/RISCV.cpp
+++ cfe/trunk/lib/Basic/Targets/RISCV.cpp
@@ -96,11 +96,12 @@
 Builder.defineMacro("__riscv_float_abi_single");
   else if (ABIName == "ilp32d" || ABIName == "lp64d")
 Builder.defineMacro("__riscv_float_abi_double");
-  else if (ABIName == "ilp32e")
-Builder.defineMacro("__riscv_abi_rve");
   else
 Builder.defineMacro("__riscv_float_abi_soft");
 
+  if (ABIName == "ilp32e")
+Builder.defineMacro("__riscv_abi_rve");
+
   if (HasM) {
 Builder.defineMacro("__riscv_mul");
 Builder.defineMacro("__riscv_div");
___
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

[Sam Elliott via Phabricator via cfe-commits]

lenary added a comment.

I don't quite understand all the details of this patch. I understand reserving 
registers that the compiler would otherwise be using as general-purpose 
registers.

But what do we do about using registers within the calling convention when 
someone says they should be reserved against compiler use? I think you're 
saying that GCC ignores that they should be fixed, and uses them anyway - it 
seems like that would cause hard-to-diagnose errors, for instance if a user 
requests fixed `x8` but requests the use of frame pointers, it seems like that 
should be an error, and yet here it might not be? I think the clang-y approach 
would be to say "you can't fix x8, we're going to use it anyway", at which 
point the "fix x8" option becomes pointless.




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

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


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] D65634: [RISCV] Default to ilp32d/lp64d in RISC-V Linux

[Sam Elliott via Phabricator via cfe-commits]

lenary accepted this revision.
lenary added a comment.
This revision is now accepted and ready to land.
Herald added a subscriber: pzheng.

I think my feeling is that this patch can land and we can change the default 
abi for baremetal targets in a follow-up patch.




Comment at: clang/lib/Driver/ToolChains/Arch/RISCV.cpp:386
+  else
+return Triple.getArch() == llvm::Triple::riscv32 ? "ilp32" : "lp64";
 }

luismarques wrote:
> luismarques wrote:
> > When I compile a bare metal GNU toolchain (using 
> > , reports GCC 8.3.0) I seem 
> > to get lp64d by default. Should we not match that behaviour?
> > 
> > ```
> > 
> > $ cat test.c
> > float foo() { return 42.0; }
> > $ riscv64-unknown-elf-gcc -O2 -S test.c
> > $ cat test.s
> > (...)
> > foo:
> > lui a5,%hi(.LC0)
> > flw fa0,%lo(.LC0)(a5)
> > (...)
> > ```
> To clarify, that's a toolchain configured with `--enable-multilib`.
I'm confused by this @luismarques 

If I do `riscv64-unknown-elf-gcc -c test.c` followed by 
`riscv64-unknown-elf-objdump -f test.o`, the flags displayed are 0x0011, 
which does not include `ELF::EF_RISCV_FLOAT_ABI_DOUBLE` (0x4), and so suggests 
gcc is using `-mabi=lp64` on baremetal elf targets. 

That said, `riscv64-unknown-elf-gcc -### -c test.c` shows it's invoking all 
subtools with `-march=rv64imafdc` and `-mabi-lp64d`. This is still with 
`--enable-multilib`, using `riscv64-unknown-elf-gcc (GCC) 8.3.0` and `GNU 
objdump (GNU Binutils) 2.32`.

I tried to look at where a default was being set in the gcc repo, and the only 
thing I can see is that the `rv64imafdc/lp64d` is the last combination to be 
generated in the multilib configuration, so they may not have explicitly chosen 
it as a default. I'm not sure. 


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

https://reviews.llvm.org/D65634



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

[PATCH] D36949: [clang] Fix tests for Emitting Single Inline Remark

[Sam Elliott via Phabricator via cfe-commits]

lenary created this revision.

This change depends on https://reviews.llvm.org/D36054 and should be landed at 
the same time.


https://reviews.llvm.org/D36949

Files:
  test/Frontend/optimization-remark-with-hotness.c
  test/Frontend/optimization-remark.c


Index: test/Frontend/optimization-remark.c
===
--- test/Frontend/optimization-remark.c
+++ test/Frontend/optimization-remark.c
@@ -42,9 +42,8 @@
 // twice.
 //
 int bar(int j) {
-// expected-remark@+4 {{foz not inlined into bar because it should never be 
inlined (cost=never)}}
 // expected-remark@+3 {{foz not inlined into bar because it should never be 
inlined (cost=never)}}
-// expected-remark@+2 {{foo should always be inlined}}
+// expected-remark@+2 {{foz not inlined into bar because it should never be 
inlined (cost=never)}}
 // expected-remark@+1 {{foo inlined into bar}}
   return foo(j, j - 2) * foz(j - 2, j);
 }
Index: test/Frontend/optimization-remark-with-hotness.c
===
--- test/Frontend/optimization-remark-with-hotness.c
+++ test/Frontend/optimization-remark-with-hotness.c
@@ -56,8 +56,7 @@
   // THRESHOLD-NOT: hotness
   // NO_PGO: '-fdiagnostics-show-hotness' requires profile-guided optimization 
information
   // NO_PGO: '-fdiagnostics-hotness-threshold=' requires profile-guided 
optimization information
-  // expected-remark@+2 {{foo should always be inlined (cost=always) (hotness: 
30)}}
-  // expected-remark@+1 {{foo inlined into bar (hotness: 30)}}
+  // expected-remark@+1 {{foo inlined into bar with cost=always}}
   sum += foo(x, x - 2);
 }
 


Index: test/Frontend/optimization-remark.c
===
--- test/Frontend/optimization-remark.c
+++ test/Frontend/optimization-remark.c
@@ -42,9 +42,8 @@
 // twice.
 //
 int bar(int j) {
-// expected-remark@+4 {{foz not inlined into bar because it should never be inlined (cost=never)}}
 // expected-remark@+3 {{foz not inlined into bar because it should never be inlined (cost=never)}}
-// expected-remark@+2 {{foo should always be inlined}}
+// expected-remark@+2 {{foz not inlined into bar because it should never be inlined (cost=never)}}
 // expected-remark@+1 {{foo inlined into bar}}
   return foo(j, j - 2) * foz(j - 2, j);
 }
Index: test/Frontend/optimization-remark-with-hotness.c
===
--- test/Frontend/optimization-remark-with-hotness.c
+++ test/Frontend/optimization-remark-with-hotness.c
@@ -56,8 +56,7 @@
   // THRESHOLD-NOT: hotness
   // NO_PGO: '-fdiagnostics-show-hotness' requires profile-guided optimization information
   // NO_PGO: '-fdiagnostics-hotness-threshold=' requires profile-guided optimization information
-  // expected-remark@+2 {{foo should always be inlined (cost=always) (hotness: 30)}}
-  // expected-remark@+1 {{foo inlined into bar (hotness: 30)}}
+  // expected-remark@+1 {{foo inlined into bar with cost=always}}
   sum += foo(x, x - 2);
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D36949: [clang] Fix tests for Emitting Single Inline Remark

[Sam Elliott via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL311347: [clang] Fix tests for Emitting Single Inline Remark 
(authored by lenary).

Repository:
  rL LLVM

https://reviews.llvm.org/D36949

Files:
  cfe/trunk/test/Frontend/optimization-remark-with-hotness.c
  cfe/trunk/test/Frontend/optimization-remark.c


Index: cfe/trunk/test/Frontend/optimization-remark.c
===
--- cfe/trunk/test/Frontend/optimization-remark.c
+++ cfe/trunk/test/Frontend/optimization-remark.c
@@ -42,9 +42,8 @@
 // twice.
 //
 int bar(int j) {
-// expected-remark@+4 {{foz not inlined into bar because it should never be 
inlined (cost=never)}}
 // expected-remark@+3 {{foz not inlined into bar because it should never be 
inlined (cost=never)}}
-// expected-remark@+2 {{foo should always be inlined}}
+// expected-remark@+2 {{foz not inlined into bar because it should never be 
inlined (cost=never)}}
 // expected-remark@+1 {{foo inlined into bar}}
   return foo(j, j - 2) * foz(j - 2, j);
 }
Index: cfe/trunk/test/Frontend/optimization-remark-with-hotness.c
===
--- cfe/trunk/test/Frontend/optimization-remark-with-hotness.c
+++ cfe/trunk/test/Frontend/optimization-remark-with-hotness.c
@@ -56,8 +56,7 @@
   // THRESHOLD-NOT: hotness
   // NO_PGO: '-fdiagnostics-show-hotness' requires profile-guided optimization 
information
   // NO_PGO: '-fdiagnostics-hotness-threshold=' requires profile-guided 
optimization information
-  // expected-remark@+2 {{foo should always be inlined (cost=always) (hotness: 
30)}}
-  // expected-remark@+1 {{foo inlined into bar (hotness: 30)}}
+  // expected-remark@+1 {{foo inlined into bar with cost=always}}
   sum += foo(x, x - 2);
 }
 


Index: cfe/trunk/test/Frontend/optimization-remark.c
===
--- cfe/trunk/test/Frontend/optimization-remark.c
+++ cfe/trunk/test/Frontend/optimization-remark.c
@@ -42,9 +42,8 @@
 // twice.
 //
 int bar(int j) {
-// expected-remark@+4 {{foz not inlined into bar because it should never be inlined (cost=never)}}
 // expected-remark@+3 {{foz not inlined into bar because it should never be inlined (cost=never)}}
-// expected-remark@+2 {{foo should always be inlined}}
+// expected-remark@+2 {{foz not inlined into bar because it should never be inlined (cost=never)}}
 // expected-remark@+1 {{foo inlined into bar}}
   return foo(j, j - 2) * foz(j - 2, j);
 }
Index: cfe/trunk/test/Frontend/optimization-remark-with-hotness.c
===
--- cfe/trunk/test/Frontend/optimization-remark-with-hotness.c
+++ cfe/trunk/test/Frontend/optimization-remark-with-hotness.c
@@ -56,8 +56,7 @@
   // THRESHOLD-NOT: hotness
   // NO_PGO: '-fdiagnostics-show-hotness' requires profile-guided optimization information
   // NO_PGO: '-fdiagnostics-hotness-threshold=' requires profile-guided optimization information
-  // expected-remark@+2 {{foo should always be inlined (cost=always) (hotness: 30)}}
-  // expected-remark@+1 {{foo inlined into bar (hotness: 30)}}
+  // expected-remark@+1 {{foo inlined into bar with cost=always}}
   sum += foo(x, x - 2);
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits