[PATCH] D112941: [clang] Add support for the new pointer authentication builtins.

[Ana Pazos via Phabricator via cfe-commits]

apazos added a comment.

The plan to push support for ptrauth_sign_constant, and 
ptrauth_type_discriminator/ptrauth_string_discriminator in a separate patch is 
good.
 This current patch is already big.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112941

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

[PATCH] D112941: [clang] Add support for the new pointer authentication builtins.

[Ana Pazos via Phabricator via cfe-commits]

apazos added inline comments.



Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:839
+def err_ptrauth_disabled :
+  Error<"pointer authentication is disabled for the current target">;
+def err_ptrauth_invalid_key :

These two error types are confusing. 
In which situation would err_ptrauth_disabled be printed?
With this patch, it is only supported with arm64e triple, all other targets it 
is unsupported.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112941

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

[PATCH] D102782: Add support for Warning Flag "-Wstack-usage="

[Ana Pazos via Phabricator via cfe-commits]

apazos added inline comments.



Comment at: clang/lib/Basic/Warnings.cpp:101
+Opt = "no-frame-larger-than=";
+
   // Check to see if this warning starts with "no-", if so, this is a

Since GCC supports the negative option -Wno-stack-usage, I think it is best to 
add the negative option Wno-frame-larger-than in the td file and alias it too, 
to avoid this C code in here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102782

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

[PATCH] D96381: [AArch64] Adding SHA3 Intrinsics support

[Ana Pazos via Phabricator via cfe-commits]

apazos added a comment.

This looks like a straightforward implementation. The only caveat is that the 
XAR immediate does not represent a lane, and hence the need for a custom 
immediate range check. Looks sensible to me.
@labrinea   and others at ARM, do have any other comment before this is merged?


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

https://reviews.llvm.org/D96381

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

[PATCH] D96381: [AArch64] Adding SHA3 Intrinsics support

[Ana Pazos via Phabricator via cfe-commits]

apazos added inline comments.



Comment at: clang/utils/TableGen/NeonEmitter.cpp:2118
 Record *R = Def->getRecord();
-if (R->getValueAsBit("isVCVT_N")) {
+if (R->getValueAsBit("isVXAR")) {
+  //VXAR takes an immediate in the range [0, 63]

Consider alphabetizing the check. move isVXAR check after isVCT_N


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96381

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

[PATCH] D95655: [AArch64] Adding Neon Sm3 & Sm4 Intrinsics

[Ana Pazos via Phabricator via cfe-commits]

apazos added inline comments.



Comment at: clang/test/CodeGen/aarch64-neon-sm4-sm3-invalid.c:8
+
+void test_range_check_vsm3tt1a(uint32x4_t a, uint32x4_t b, uint32x4_t c) {
+  // CHECK-RANGE: error: argument value 5 is outside the valid range [0, 3]

Consider renaming the test with range-check suffix so it is clear it is 
checking for invalid  range for the immediate
In addition, consider rewriting it with //expected-error checks



Comment at: clang/test/CodeGen/aarch64-neon-sm4-sm3.c:5
+
+// RUN: not %clang_cc1 -triple aarch64-linux-gnu -target-feature +neon \
+// RUN: -S -emit-llvm -o - %s 2>&1 | FileCheck --check-prefix=CHECK-NO-CRYPTO 
%s

The test is written for aarch64 triple, in addition @t.p.northover might want 
to add arm64 triple checks. Tim what do you think?


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

https://reviews.llvm.org/D95655

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

[PATCH] D93428: [AArch64] Add bti note property when compiling asm files with -mbranch-protection=bti

[Ana Pazos via Phabricator via cfe-commits]

apazos added a comment.

Can you confirm when the GNU toolchain will have this flag supported in their 
assembler? Compatibility between LLVM and GNU toolchains is important.

Stephen - I think we can abandon this review. Users will need to be made aware 
of this additional assembler flag when building .s files with BTI enabled.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93428

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

[PATCH] D93428: [AArch64] Add bti note property when compiling asm files with -mbranch-protection=bti

[Ana Pazos via Phabricator via cfe-commits]

apazos added a comment.

Thanks Daniel for the explanation. Was the support added for GNU assembler as 
well? Is it the same flag name?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93428

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

[PATCH] D93428: [AArch64] Add bti note property when compiling asm files with -mbranch-protection=bti

[Ana Pazos via Phabricator via cfe-commits]

apazos added a comment.

Thanks for clarifying - so the property is being set for C/C++ files but not 
for assembly files. I think it should be set automatically for both when one 
uses clang driver to compile/assemble.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93428

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

[PATCH] D93428: [AArch64] Add bti note property when compiling asm files with -mbranch-protection=bti

[Ana Pazos via Phabricator via cfe-commits]

apazos added a comment.

So the intention here is to generate the property when BTI branch protection is 
enabled, to guarantee the generate .o indeed has the property set, without 
requiring to pass the flag -mmark-bti-property explicitly. This is convenient 
for users.

Or is there a clear reason why one would enable BIT branch protection and not 
have the property set? What would be that use case?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93428

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

[PATCH] D84414: [RISCV] Support Shadow Call Stack

[Ana Pazos via Phabricator via cfe-commits]

apazos added inline comments.



Comment at: llvm/lib/Target/RISCV/RISCVFrameLowering.cpp:95
+  .addReg(RISCV::X18)
+  .addImm(0);
+}

There are thee things to observe here and other reviewers might have some 
additional comments:

- RISC-V does not have a reserved platform register like AAch64. The patch uses 
one of the RISC-V callee saved registers, x18, which happens to coincide with 
AArch64's register. It is possible to select another register, and additional 
checks for the flag combo "-fsanitize=shadow-call-stack -ffixed-" will have 
to be added.

- The  return address is saved on both the SCS (whose location is 
protected/hidden) and also in the regular stack. But the return from a function 
uses the value saved on SCS. The understanding is that not saving it in the 
regular stack can impact debugging.

- The SCS is ascending, while the regular stack, by RISC-V convention, is 
descending. The SCS is not used for passing parameters between calls like the 
regular stack, so it seems to be ok. But this can be changed too. AArch64 's 
SCS is also ascending.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84414



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

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

[Ana Pazos via Phabricator via cfe-commits]

apazos added a comment.

Thanks Shiva, making it alias of -G makes sense, LGTM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D57497



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

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

[Ana Pazos via Phabricator via cfe-commits]

apazos added a comment.

Shiva, how about making the flag small-data-limit alias of 
-msmall-data-threshold?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D57497



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

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

[Ana Pazos via Phabricator via cfe-commits]

apazos added a comment.

Thanks Shiva, I res-ynced and rebuilt the patch. It is working fine.

I see there is a msmall-data-threshold flag used by Mips and Hexagon, and now 
we are adding a new flag msmall-data-limit. Should't we reuse the flag?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D57497



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

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

[Ana Pazos via Phabricator via cfe-commits]

apazos added a comment.

Shiva, I am not sure how the SDataLimit is being honored in LTO mode. 
Where does getModuleMetadata get called?
If the SelectSectionForGlobal api is called without getModuleMetadata being 
called first, it will use the default 8 instead of honoring the SDataLimit.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D57497



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

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

[Ana Pazos via Phabricator via cfe-commits]

apazos added a comment.

Shiva, I see a warning always  being printed:

'+small-data-limit=' is not a recognized feature for this target (ignoring 
feature)

This is because it is being passed down as a target feature.

Might be good to add a test case to make sure the SmallDataLimit module flag is 
created, no target feature is passed, and that no warnings are printed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D57497



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

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

[Ana Pazos via Phabricator via cfe-commits]

apazos added a comment.
Herald added subscribers: evandro, luismarques, sameer.abuasal, pzheng, 
s.egerton, lenary.

Shiva, we forgot about this patch. Can you rebase it so we move on with merging.


Repository:
  rC Clang

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

https://reviews.llvm.org/D57497



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

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

[Ana Pazos via Phabricator via cfe-commits]

apazos added a comment.

Lewis, your latest patch looks good, we just had another run with no new 
failures. But we know it will have issues with -g. So I think we should not 
merge it yet. Do you have a version of the patch that creates the labels for 
the compiler-generated save/restore lib calls, so that this optimization does 
not depend on D71593 ? We could merge that 
version then, and when D71593  is accepted, 
you just have to rework/remove the label generation part of the patch.


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

[Ana Pazos via Phabricator via cfe-commits]

apazos added a comment.

Lewis, is the patch final? It would be good to merge it before the 10.0 release 
branch creation on Jan 15th


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

[Ana Pazos via Phabricator via cfe-commits]

apazos added a comment.

Lewis, try rebasing it, not applying cleanly nor https://reviews.llvm.org/D62190


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

[Ana Pazos via Phabricator via cfe-commits]

apazos added a comment.

Thanks Lewis, the runs are looking good, no failures, and good code size 
savings (average 3%)


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

[Ana Pazos via Phabricator via cfe-commits]

apazos added a comment.



> Is it worth trying to disallow tail call optimization completely if this flag 
> is enabled? I'm not sure what GCC does exactly. but this seems to be the 
> behaviour.

I had reported above that I have already run that test:  with 
"-fno-optimize-sibling-calls -msave-restore", i.e., disabling tail calls when 
m-save-restore is enabled.

But it seems a better solution is to optimistically apply -msave-restore when 
there are no tail calls of any type in a function,  instead of disabling tail 
calls completed. Let tail call optimization prevail over msave-restore. No LLVM 
target is disabling tail calls.

So you can update the patch according to this solution plan.


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

[Ana Pazos via Phabricator via cfe-commits]

apazos added a comment.

Yes Eli thanks for pointing out  there are more scenarios that can fail.
It looks like the best solution is to permit both flags on, but then bail out 
from doing this transformation if there is any type of tail call already in the 
function.
This way we avoid messing with reverting tail calls back to regular calls.


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

[Ana Pazos via Phabricator via cfe-commits]

apazos added a comment.

Here is the bugpoint-reduced test case for the SPEC failure when enabling 
-msave-restore and allowing tail calls:

Run the command llc test.ll -mattr=+save-restore -o out.s

You will see the code generated is wrong:

  tail__riscv_restore_2
   jr  a5

target datalayout = "e-m:e-p:32:32-i64:64-n32-S128"
target triple = "riscv32-unknown-linux-gnu"

%struct.1_s = type { i8*, i8*, i8*}
%struct.2_s = type { i8*, i8*, i8*}

declare dso_local void @test2() local_unnamed_addr
declare dso_local i32 @test3(%struct.2_s* nonnull %a, %struct.1_s** %b) 
local_unnamed_addr

define dso_local void @test1(%struct.2_s* %a, %struct.1_s** %b) 
local_unnamed_addr  {
entry:

  br i1 undef, label %if.end2, label %if.then

if.then:  ; preds = %entry

  tail call void @test2()
  br label %if.end2

if.end2:  ; preds = %if.then, %entry

  %call3 = tail call i32 null(%struct.2_s* nonnull %a, %struct.1_s** %b)
  ret void

}

To summarize the issues we have detected so far:

1. Bug1: When transforming a tail call back into a regular call, the 
transformation is not correctly preserving the original call's return values.

I fixed this by conservatively adding RegState::Implicit definitions for each 
possible integer and FP return registers, when defining the riscv_restore tail 
call.

2. Bug2: test case provided above. Please Lewis take a look at how this case 
can be fixed.

3. I have also run the tests with "-fno-optimize-sibling-calls -msave-restore", 
i.e., disabling tail calls when m-save-restore is enabled.

With this config, issues (1) and (2) do not happen.
There is some loss in code size savings when using fno-optimize-sibling-calls.
But still we see code size savings benefit from -msave-restore.


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

[Ana Pazos via Phabricator via cfe-commits]

apazos added inline comments.



Comment at: llvm/lib/Target/RISCV/RISCVFrameLowering.cpp:631
+  MachineBasicBlock::iterator NewMI =
+  BuildMI(MBB, MI, DL, TII.get(RISCV::PseudoCALL))
+  .add(MI->getOperand(0));

apazos wrote:
> Where are we making sure the PseudoCALL result in a0 is alive through the 
> riscv_restore call?
I did not find any other target that transforms a tail call back into a regular 
call.

The issue with doing this is that we don't have info about the return value of 
the original call.

If anyone knows how to do it, please give me some pointers.

I tried to fix this problem by adding implicit operands to  the riscv_restore 
tail call that are all the possible return value registers:
BuildMI(MBB, MI, DL, TII.get(RISCV::PseudoTAIL))
.addExternalSymbol(RestoreLibCall, RISCVII::MO_CALL)
-.setMIFlag(MachineInstr::FrameDestroy);
+.setMIFlag(MachineInstr::FrameDestroy)
+.addReg(RISCV::X10, RegState::Implicit)
+.addReg(RISCV::X11, RegState::Implicit)
+.addReg(RISCV::F10_F, RegState::Implicit)
+.addReg(RISCV::F11_F, RegState::Implicit)

With this change, some perennial tests now pass.

But there are still failures to be analyzed in SPEC.


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

[Ana Pazos via Phabricator via cfe-commits]

apazos added inline comments.



Comment at: llvm/lib/Target/RISCV/RISCVFrameLowering.cpp:631
+  MachineBasicBlock::iterator NewMI =
+  BuildMI(MBB, MI, DL, TII.get(RISCV::PseudoCALL))
+  .add(MI->getOperand(0));

Where are we making sure the PseudoCALL result in a0 is alive through the 
riscv_restore call?


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

[Ana Pazos via Phabricator via cfe-commits]

apazos added a comment.

Lewis, with this patch we see less failures. But still some tests in SPEC and 
perennial test suites are failing.

Pengxuan and I are trying to triage the failures.

Here is what we see in one of the failed tests:

Code right before rologue/Epilogue Insertion & Frame Finalization:

  %call39 = tail call i32 @somefunction()
  ret i32 %call39

Code after Prologue/Epilogue Insertion & Frame Finalization:

  PseudoCALL target-flags() @somefunction, implicit-def $x1, implicit 
$x2
  $x10 = frame-destroy LUI 1
  $x10 = frame-destroy ADDI killed $x10, -1712
  $x2 = frame-destroy ADD $x2, killed $x10
  CFI_INSTRUCTION def_cfa_offset 0
  frame-destroy PseudoTAIL target-flags() &__riscv_restore_12, 
implicit $x2

x10 is the return value and it is being overwritten.

Is this patch self contained? Or does it depend on another patch under review?

For the tests that are passing, I see better code size reduction with C 
extension enabled (testing RV32IMAC) than with the machine outliner.


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

[Ana Pazos via Phabricator via cfe-commits]

apazos added a comment.

Thanks for the patch update. I will launch some new correctness runs.


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] D57497: [RISCV] Passing small data limitation value to RISCV backend

[Ana Pazos via Phabricator via cfe-commits]

apazos added inline comments.



Comment at: lib/Driver/ToolChains/Clang.cpp:1813
+}
+  } else if (Arg *A = Args.getLastArg(options::OPT_G)) {
+SmallDataLimit = A->getValue();

Why do you we need to set a default? It will cause the optimization to be on 
now, and I thought we want to first prioritize the globals we want to consider 
for the optimization.
I think you should check for msmall-data-limit flag occurring with G,  fpic and 
mcmodel flags and print warning in all cases.


Repository:
  rC Clang

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

https://reviews.llvm.org/D57497



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

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

[Ana Pazos via Phabricator via cfe-commits]

apazos added inline comments.
Herald added subscribers: benna, psnobl.



Comment at: lib/Driver/ToolChains/Clang.cpp:1803
+
+  // Forward the -msmall-data-limit= option.
+  if (Arg *A = Args.getLastArg(options::OPT_G)) {

Might be simpler to just set it to 0, and if G is present in the command line, 
and the other flags are not present, then you change the value. Also, might be 
good to print a warning to let user know -G is ignored?



Comment at: lib/Driver/ToolChains/RISCVToolchain.cpp:117
 
+  if (D.isUsingLTO()) {
+assert(!Inputs.empty() && "Must have at least one input.");

This change is not part of this patch.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D57497



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

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

[Ana Pazos via Phabricator via cfe-commits]

apazos added a comment.

If this is a target flag in GCC, shouldn't we make it a LLVM Target feature and 
pass it as -mattr, just like done for mrelax?


Repository:
  rC Clang

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

https://reviews.llvm.org/D57497



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

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

[Ana Pazos via Phabricator via cfe-commits]

apazos added a comment.

So Eli is concerned we might end up with many globals in the small data section 
or not picking the best candidates if we pass -G to all files in LTO.
I don’t know if anyone has experimented with a heuristic to selectively pick 
which globals and of which size will be allowed to go into the  small data 
section.
Simon, do you have any insight?
Shiva, maybe for now we don’t pass the flag to LTO. But I think you got the 
correct mechanism. The only other suggestion I have is to add a RISC-V specific 
function to avoid too much RISC-V specific code in 
gnutools::Linker::constructJob. You just check the triple and call something 
like toolchains::RISCVToolChain::AddGoldPluginAdditionalFlags.


Repository:
  rC Clang

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

https://reviews.llvm.org/D57497



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

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

[Ana Pazos via Phabricator via cfe-commits]

apazos added a comment.

I don't see -plugin-opt=-riscv-ssection-threshold=.. being passed.
tools::gnutools::Linker::ConstructJob is being invoked with target 
riscv32-unknown-linux-gnu
It has to work for riscv32-unknown-linux-gnu and riscv32-unknown-elf


Repository:
  rC Clang

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

https://reviews.llvm.org/D57497



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

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

[Ana Pazos via Phabricator via cfe-commits]

apazos added a comment.

Hi Shiva, I will check, but I think you need to also modify gnutools:Linker 
because riscv::Linker is called for baremetal. I think you need in both places.
The way I check is by invoking -flto -v from clang and look at the arguments 
passed to the compiler and linker


Repository:
  rC Clang

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

https://reviews.llvm.org/D57497



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

[PATCH] D57497: [RISCV] Passing -G value to RISCV backend

[Ana Pazos via Phabricator via cfe-commits]

apazos added a comment.

Hi Shiva, I think you need to check for and pass  along the -G option to the 
linker (gnutools::Linker and RISCV::Linker) and will be available for LTO. 
Check Hexagon, it passes the threshold value to the assembler (via -gpsize)  
and linker (via -G).


Repository:
  rC Clang

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

https://reviews.llvm.org/D57497



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

[PATCH] D37624: add support for -fno-instrument-functions and -finstrument-functions-exclude-{file, function}-list= to match gcc options.

[Ana Pazos via Phabricator via cfe-commits]

apazos added a comment.
Herald added a subscriber: erik.pilkington.

Hello folks, is there a plan to merge this feature still?


https://reviews.llvm.org/D37624



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

[PATCH] D48412: [RISCV] Add support for interrupt attribute

[Ana Pazos via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC338045:  [RISCV] Add support for interrupt attribute 
(authored by apazos, committed by ).
Herald added a subscriber: jrtc27.

Repository:
  rC Clang

https://reviews.llvm.org/D48412

Files:
  include/clang/Basic/Attr.td
  include/clang/Basic/AttrDocs.td
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/CodeGen/TargetInfo.cpp
  lib/Sema/SemaDeclAttr.cpp
  test/Sema/riscv-interrupt-attr.c
  test/Sema/riscv-interrupt-attr.cpp

Index: include/clang/Basic/AttrDocs.td
===
--- include/clang/Basic/AttrDocs.td
+++ include/clang/Basic/AttrDocs.td
@@ -1501,6 +1501,29 @@
   }];
 }
 
+def RISCVInterruptDocs : Documentation {
+  let Category = DocCatFunction;
+  let Heading = "interrupt (RISCV)";
+  let Content = [{
+Clang supports the GNU style ``__attribute__((interrupt))`` attribute on RISCV
+targets. This attribute may be attached to a function definition and instructs
+the backend to generate appropriate function entry/exit code so that it can be
+used directly as an interrupt service routine.
+
+Permissible values for this parameter are ``user``, ``supervisor``,
+and ``machine``. If there is no parameter, then it defaults to machine.
+
+Repeated interrupt attribute on the same declaration will cause a warning
+to be emitted. In case of repeated declarations, the last one prevails.
+
+Refer to:
+https://gcc.gnu.org/onlinedocs/gcc/RISC-V-Function-Attributes.html
+https://riscv.org/specifications/privileged-isa/
+The RISC-V Instruction Set Manual Volume II: Privileged Architecture
+Version 1.10.
+  }];
+}
+
 def AVRInterruptDocs : Documentation {
   let Category = DocCatFunction;
   let Heading = "interrupt (AVR)";
Index: include/clang/Basic/DiagnosticSemaKinds.td
===
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -274,14 +274,22 @@
"MIPS 'interrupt' attribute only applies to functions that have "
"%select{no parameters|a 'void' return type}0">,
InGroup;
+def warn_riscv_repeated_interrupt_attribute : Warning<
+  "repeated RISC-V 'interrupt' attribute">, InGroup;
+def note_riscv_repeated_interrupt_attribute : Note<
+  "repeated RISC-V 'interrupt' attribute is here">;
+def warn_riscv_interrupt_attribute : Warning<
+   "RISC-V 'interrupt' attribute only applies to functions that have "
+   "%select{no parameters|a 'void' return type}0">,
+   InGroup;
 def warn_unused_parameter : Warning<"unused parameter %0">,
   InGroup, DefaultIgnore;
 def warn_unused_variable : Warning<"unused variable %0">,
   InGroup, DefaultIgnore;
 def warn_unused_local_typedef : Warning<
   "unused %select{typedef|type alias}0 %1">,
   InGroup, DefaultIgnore;
-def warn_unused_property_backing_ivar : 
+def warn_unused_property_backing_ivar :
   Warning<"ivar %0 which backs the property is not "
   "referenced in this property's accessor">,
   InGroup, DefaultIgnore;
Index: include/clang/Basic/Attr.td
===
--- include/clang/Basic/Attr.td
+++ include/clang/Basic/Attr.td
@@ -308,6 +308,7 @@
 def TargetMips32 : TargetArch<["mips", "mipsel"]>;
 def TargetAnyMips : TargetArch<["mips", "mipsel", "mips64", "mips64el"]>;
 def TargetMSP430 : TargetArch<["msp430"]>;
+def TargetRISCV : TargetArch<["riscv32", "riscv64"]>;
 def TargetX86 : TargetArch<["x86"]>;
 def TargetAnyX86 : TargetArch<["x86", "x86_64"]>;
 def TargetWindows : TargetArch<["x86", "x86_64", "arm", "thumb", "aarch64"]> {
@@ -1374,6 +1375,17 @@
   let Documentation = [MicroMipsDocs];
 }
 
+def RISCVInterrupt : InheritableAttr, TargetSpecificAttr {
+  let Spellings = [GCC<"interrupt">];
+  let Subjects = SubjectList<[Function]>;
+  let Args = [EnumArgument<"Interrupt", "InterruptType",
+   ["user", "supervisor", "machine"],
+   ["user", "supervisor", "machine"],
+   1>];
+  let ParseKind = "Interrupt";
+  let Documentation = [RISCVInterruptDocs];
+}
+
 // This is not a TargetSpecificAttr so that is silently accepted and
 // ignored on other targets as encouraged by the OpenCL spec.
 //
Index: test/Sema/riscv-interrupt-attr.c
===
--- test/Sema/riscv-interrupt-attr.c
+++ test/Sema/riscv-interrupt-attr.c
@@ -0,0 +1,62 @@
+// RUN: %clang_cc1 -triple riscv32-unknown-elf -emit-llvm -DCHECK_IR < %s| FileCheck %s
+// RUN: %clang_cc1 -triple riscv64-unknown-elf -emit-llvm -DCHECK_IR < %s| FileCheck %s
+// RUN: %clang_cc1 %s -triple riscv32-unknown-elf -verify -fsyntax-only
+// RUN: %clang_cc1 %s -triple riscv64-unknown-elf -verify -fsyntax-only
+
+#if defined(CHECK_IR)
+// CHECK-LABEL:  @foo_user() #0
+// CHECK: ret void
+__attribute__((interrupt("user"))) void foo_user(void) {}
+// CHECK-LABEL:  @foo_supervisor() #1
+// CHECK: 

[PATCH] D48412: [RISCV] Add support for interrupt attribute

[Ana Pazos via Phabricator via cfe-commits]

apazos updated this revision to Diff 156343.
apazos added a comment.

- Rebased the patch.
- Udpated test case to check for IR attribute.


https://reviews.llvm.org/D48412

Files:
  include/clang/Basic/Attr.td
  include/clang/Basic/AttrDocs.td
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/CodeGen/TargetInfo.cpp
  lib/Sema/SemaDeclAttr.cpp
  test/Sema/riscv-interrupt-attr.c
  test/Sema/riscv-interrupt-attr.cpp

Index: test/Sema/riscv-interrupt-attr.cpp
===
--- /dev/null
+++ test/Sema/riscv-interrupt-attr.cpp
@@ -0,0 +1,16 @@
+// RUN: %clang_cc1 -x c++ -triple riscv32-unknown-elf -emit-llvm  -DCHECK_IR < %s | FileCheck %s
+// RUN: %clang_cc1 -x c++ -triple riscv64-unknown-elf -emit-llvm  -DCHECK_IR < %s | FileCheck %s
+// RUN: %clang_cc1 %s -triple riscv32-unknown-elf -verify -fsyntax-only
+// RUN: %clang_cc1 %s -triple riscv64-unknown-elf -verify -fsyntax-only
+
+#if defined(CHECK_IR)
+// CHECK-LABEL: @_Z11foo_defaultv() #0
+// CHECK: ret void
+[[gnu::interrupt]] void foo_default() {}
+// CHECK: attributes #0
+// CHECK: "interrupt"="machine"
+#else
+[[gnu::interrupt]] [[gnu::interrupt]] void foo1() {} // expected-warning {{repeated RISC-V 'interrupt' attribute}} \
+ // expected-note {{repeated RISC-V 'interrupt' attribute is here}}
+[[gnu::interrupt]] void foo2() {}
+#endif
Index: test/Sema/riscv-interrupt-attr.c
===
--- /dev/null
+++ test/Sema/riscv-interrupt-attr.c
@@ -0,0 +1,62 @@
+// RUN: %clang_cc1 -triple riscv32-unknown-elf -emit-llvm -DCHECK_IR < %s| FileCheck %s
+// RUN: %clang_cc1 -triple riscv64-unknown-elf -emit-llvm -DCHECK_IR < %s| FileCheck %s
+// RUN: %clang_cc1 %s -triple riscv32-unknown-elf -verify -fsyntax-only
+// RUN: %clang_cc1 %s -triple riscv64-unknown-elf -verify -fsyntax-only
+
+#if defined(CHECK_IR)
+// CHECK-LABEL:  @foo_user() #0
+// CHECK: ret void
+__attribute__((interrupt("user"))) void foo_user(void) {}
+// CHECK-LABEL:  @foo_supervisor() #1
+// CHECK: ret void
+__attribute__((interrupt("supervisor"))) void foo_supervisor(void) {}
+// CHECK-LABEL:  @foo_machine() #2
+// CHECK: ret void
+__attribute__((interrupt("machine"))) void foo_machine(void) {}
+// CHECK-LABEL:  @foo_default() #2
+// CHECK: ret void
+__attribute__((interrupt())) void foo_default(void) {}
+// CHECK-LABEL:  @foo_default2() #2
+// CHECK: ret void
+__attribute__((interrupt())) void foo_default2(void) {}
+// CHECK: attributes #0
+// CHECK: "interrupt"="user"
+// CHECK: attributes #1
+// CHECK: "interrupt"="supervisor"
+// CHECK: attributes #2
+// CHECK: "interrupt"="machine"
+#else
+struct a { int b; };
+
+struct a test __attribute__((interrupt)); // expected-warning {{'interrupt' attribute only applies to functions}}
+
+__attribute__((interrupt("USER"))) void foo1(void) {} // expected-warning {{'interrupt' attribute argument not supported: USER}}
+
+__attribute__((interrupt("user", 1))) void foo2(void) {} // expected-error {{'interrupt' attribute takes no more than 1 argument}}
+
+__attribute__((interrupt)) int foo3(void) {return 0;} // expected-warning {{RISC-V 'interrupt' attribute only applies to functions that have a 'void' return type}}
+
+__attribute__((interrupt())) void foo4();
+__attribute__((interrupt())) void foo4() {};
+
+__attribute__((interrupt())) void foo5(int a) {} // expected-warning {{RISC-V 'interrupt' attribute only applies to functions that have no parameters}}
+
+__attribute__((interrupt("user"), interrupt("supervisor"))) void foo6(void) {} // expected-warning {{repeated RISC-V 'interrupt' attribute}} \
+  // expected-note {{repeated RISC-V 'interrupt' attribute is here}}
+
+__attribute__((interrupt, interrupt)) void foo7(void) {} // expected-warning {{repeated RISC-V 'interrupt' attribute}} \
+ // expected-note {{repeated RISC-V 'interrupt' attribute is here}}
+
+__attribute__((interrupt(""))) void foo8(void) {} // expected-warning {{'interrupt' attribute argument not supported}}
+
+__attribute__((interrupt("user"))) void foo9(void);
+__attribute__((interrupt("supervisor"))) void foo9(void);
+__attribute__((interrupt("machine"))) void foo9(void);
+
+__attribute__((interrupt("user"))) void foo10(void) {}
+__attribute__((interrupt("supervisor"))) void foo11(void) {}
+__attribute__((interrupt("machine"))) void foo12(void) {}
+__attribute__((interrupt())) void foo13(void) {}
+__attribute__((interrupt)) void foo14(void) {}
+#endif
+
Index: lib/Sema/SemaDeclAttr.cpp
===
--- lib/Sema/SemaDeclAttr.cpp
+++ lib/Sema/SemaDeclAttr.cpp
@@ -5328,6 +5328,64 @@
   handleSimpleAttribute(S, D, AL);
 }
 
+
+static void handleRISCVInterruptAttr(Sema , Decl *D,
+ const ParsedAttr ) {
+  // Warn about repeated attributes.
+  if (const auto *A = D->getAttr()) {
+

[PATCH] D48412: [RISCV] Add support for interrupt attribute

[Ana Pazos via Phabricator via cfe-commits]

apazos added a comment.

Hi Aaron, I have commit rights and will commit if after Alex and other RISC-V 
colleagues get a chance to review too.


https://reviews.llvm.org/D48412



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

[PATCH] D48412: [RISCV] Add support for interrupt attribute

[Ana Pazos via Phabricator via cfe-commits]

apazos updated this revision to Diff 154447.
apazos added a comment.

Thanks Aaron, I appreciate you taking the time to review. 
I have updated the test and removed the warning as discussed.


https://reviews.llvm.org/D48412

Files:
  include/clang/Basic/Attr.td
  include/clang/Basic/AttrDocs.td
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/CodeGen/TargetInfo.cpp
  lib/Sema/SemaDeclAttr.cpp
  test/Sema/riscv-interrupt-attr.c
  test/Sema/riscv-interrupt-attr.cpp

Index: test/Sema/riscv-interrupt-attr.cpp
===
--- /dev/null
+++ test/Sema/riscv-interrupt-attr.cpp
@@ -0,0 +1,7 @@
+// RUN: %clang_cc1 %s -triple riscv32-unknown-elf -verify -fsyntax-only
+// RUN: %clang_cc1 %s -triple riscv64-unknown-elf -verify -fsyntax-only
+
+[[gnu::interrupt]] [[gnu::interrupt]] void foo1() {} // expected-warning {{repeated RISC-V 'interrupt' attribute}} \
+ // expected-note {{repeated RISC-V 'interrupt' attribute is here}}
+[[gnu::interrupt]] void foo2() {}
+
Index: test/Sema/riscv-interrupt-attr.c
===
--- /dev/null
+++ test/Sema/riscv-interrupt-attr.c
@@ -0,0 +1,36 @@
+// RUN: %clang_cc1 %s -triple riscv32-unknown-elf -verify -fsyntax-only
+// RUN: %clang_cc1 %s -triple riscv64-unknown-elf -verify -fsyntax-only
+
+struct a { int b; };
+
+struct a test __attribute__((interrupt)); // expected-warning {{'interrupt' attribute only applies to functions}}
+
+__attribute__((interrupt("USER"))) void foo1(void) {} // expected-warning {{'interrupt' attribute argument not supported: USER}}
+
+__attribute__((interrupt("user", 1))) void foo2(void) {} // expected-error {{'interrupt' attribute takes no more than 1 argument}}
+
+__attribute__((interrupt)) int foo3(void) {return 0;} // expected-warning {{RISC-V 'interrupt' attribute only applies to functions that have a 'void' return type}}
+
+__attribute__((interrupt())) void foo4();
+__attribute__((interrupt())) void foo4() {};
+
+__attribute__((interrupt())) void foo5(int a) {} // expected-warning {{RISC-V 'interrupt' attribute only applies to functions that have no parameters}}
+
+__attribute__((interrupt("user"), interrupt("supervisor"))) void foo6(void) {} // expected-warning {{repeated RISC-V 'interrupt' attribute}} \
+  // expected-note {{repeated RISC-V 'interrupt' attribute is here}}
+
+__attribute__((interrupt, interrupt)) void foo7(void) {} // expected-warning {{repeated RISC-V 'interrupt' attribute}} \
+ // expected-note {{repeated RISC-V 'interrupt' attribute is here}}
+
+__attribute__((interrupt(""))) void foo8(void) {} // expected-warning {{'interrupt' attribute argument not supported}}
+
+__attribute__((interrupt("user"))) void foo9(void);
+__attribute__((interrupt("supervisor"))) void foo9(void);
+__attribute__((interrupt("machine"))) void foo9(void);
+
+__attribute__((interrupt("user"))) void foo10(void) {}
+__attribute__((interrupt("supervisor"))) void foo11(void) {}
+__attribute__((interrupt("machine"))) void foo12(void) {}
+__attribute__((interrupt())) void foo13(void) {}
+__attribute__((interrupt)) void foo14(void) {}
+
Index: lib/Sema/SemaDeclAttr.cpp
===
--- lib/Sema/SemaDeclAttr.cpp
+++ lib/Sema/SemaDeclAttr.cpp
@@ -5266,6 +5266,63 @@
   handleSimpleAttribute(S, D, AL);
 }
 
+static void handleRISCVInterruptAttr(Sema , Decl *D,
+ const AttributeList ) {
+  // Warn about repeated attributes.
+  if (const auto *A = D->getAttr()) {
+S.Diag(AL.getRange().getBegin(),
+  diag::warn_riscv_repeated_interrupt_attribute);
+S.Diag(A->getLocation(), diag::note_riscv_repeated_interrupt_attribute);
+return;
+  }
+
+  // Check the attribute argument. Argument is optional.
+  if (!checkAttributeAtMostNumArgs(S, AL, 1))
+return;
+
+  StringRef Str;
+  SourceLocation ArgLoc;
+
+  // 'machine'is the default interrupt mode.
+  if (AL.getNumArgs() == 0)
+Str = "machine";
+  else if (!S.checkStringLiteralArgumentAttr(AL, 0, Str, ))
+return;
+
+  // Semantic checks for a function with the 'interrupt' attribute:
+  // - Must be a function.
+  // - Must have no parameters.
+  // - Must have the 'void' return type.
+  // - The attribute itself must either have no argument or one of the
+  //   valid interrupt types, see [RISCVInterruptDocs].
+
+  if (D->getFunctionType() == nullptr) {
+S.Diag(D->getLocation(), diag::warn_attribute_wrong_decl_type)
+  << "'interrupt'" << ExpectedFunction;
+return;
+  }
+
+  if (hasFunctionProto(D) && getFunctionOrMethodNumParams(D) != 0) {
+S.Diag(D->getLocation(), diag::warn_riscv_interrupt_attribute) << 0;
+return;
+  }
+
+  if (!getFunctionOrMethodResultType(D)->isVoidType()) {
+S.Diag(D->getLocation(), diag::warn_riscv_interrupt_attribute) << 1;
+return;
+  }
+
+  

[PATCH] D48412: [RISCV] Add support for interrupt attribute

[Ana Pazos via Phabricator via cfe-commits]

apazos updated this revision to Diff 154322.
apazos added a comment.

Made the check/warning for prototype explicit.


https://reviews.llvm.org/D48412

Files:
  include/clang/Basic/Attr.td
  include/clang/Basic/AttrDocs.td
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/CodeGen/TargetInfo.cpp
  lib/Sema/SemaDeclAttr.cpp
  test/Sema/riscv-interrupt-attr.c
  test/Sema/riscv-interrupt-attr.cpp

Index: test/Sema/riscv-interrupt-attr.cpp
===
--- /dev/null
+++ test/Sema/riscv-interrupt-attr.cpp
@@ -0,0 +1,7 @@
+// RUN: %clang_cc1 %s -triple riscv32-unknown-elf -verify -fsyntax-only
+// RUN: %clang_cc1 %s -triple riscv64-unknown-elf -verify -fsyntax-only
+
+[[gnu::interrupt]] [[gnu::interrupt]] void foo1() {} // expected-warning {{repeated RISC-V 'interrupt' attribute}} \
+ // expected-note {{repeated RISC-V 'interrupt' attribute is here}}
+[[gnu::interrupt]] void foo2() {}
+
Index: test/Sema/riscv-interrupt-attr.c
===
--- /dev/null
+++ test/Sema/riscv-interrupt-attr.c
@@ -0,0 +1,37 @@
+// RUN: %clang_cc1 %s -triple riscv32-unknown-elf -verify -fsyntax-only
+// RUN: %clang_cc1 %s -triple riscv64-unknown-elf -verify -fsyntax-only
+
+struct a { int b; };
+
+struct a test __attribute__((interrupt)); // expected-warning {{'interrupt' attribute only applies to functions}}
+
+__attribute__((interrupt("USER"))) void foo1(void) {} // expected-warning {{'interrupt' attribute argument not supported: USER}}
+
+__attribute__((interrupt("user", 1))) void foo2(void) {} // expected-error {{'interrupt' attribute takes no more than 1 argument}}
+
+__attribute__((interrupt)) int foo3(void) {return 0;} // expected-warning {{RISC-V 'interrupt' attribute only applies to functions that have a 'void' return type}}
+
+__attribute__((interrupt())) int foo4(); // expected-warning {{RISC-V 'interrupt' attribute only applies to functions that have a prototype}}
+
+__attribute__((interrupt())) int foo4() {} // expected-warning {{RISC-V 'interrupt' attribute only applies to functions that have a prototype}}
+
+__attribute__((interrupt())) void foo5(int a) {} // expected-warning {{RISC-V 'interrupt' attribute only applies to functions that have no parameters}}
+
+__attribute__((interrupt("user"), interrupt("supervisor"))) void foo6(void) {} // expected-warning {{repeated RISC-V 'interrupt' attribute}} \
+  // expected-note {{repeated RISC-V 'interrupt' attribute is here}}
+
+__attribute__((interrupt, interrupt)) void foo7(void) {} // expected-warning {{repeated RISC-V 'interrupt' attribute}} \
+ // expected-note {{repeated RISC-V 'interrupt' attribute is here}}
+
+__attribute__((interrupt(""))) void foo8(void) {} // expected-warning {{'interrupt' attribute argument not supported}}
+
+__attribute__((interrupt("user"))) void foo9(void);
+__attribute__((interrupt("supervisor"))) void foo9(void);
+__attribute__((interrupt("machine"))) void foo9(void);
+
+__attribute__((interrupt("user"))) void foo10(void) {}
+__attribute__((interrupt("supervisor"))) void foo11(void) {}
+__attribute__((interrupt("machine"))) void foo12(void) {}
+__attribute__((interrupt())) void foo13(void) {}
+__attribute__((interrupt)) void foo14(void) {}
+
Index: lib/Sema/SemaDeclAttr.cpp
===
--- lib/Sema/SemaDeclAttr.cpp
+++ lib/Sema/SemaDeclAttr.cpp
@@ -5266,6 +5266,68 @@
   handleSimpleAttribute(S, D, AL);
 }
 
+static void handleRISCVInterruptAttr(Sema , Decl *D,
+ const AttributeList ) {
+  // Warn about repeated attributes.
+  if (const auto *A = D->getAttr()) {
+S.Diag(AL.getRange().getBegin(),
+  diag::warn_riscv_repeated_interrupt_attribute);
+S.Diag(A->getLocation(), diag::note_riscv_repeated_interrupt_attribute);
+return;
+  }
+
+  // Check the attribute argument. Argument is optional.
+  if (!checkAttributeAtMostNumArgs(S, AL, 1))
+return;
+
+  StringRef Str;
+  SourceLocation ArgLoc;
+
+  // 'machine'is the default interrupt mode.
+  if (AL.getNumArgs() == 0)
+Str = "machine";
+  else if (!S.checkStringLiteralArgumentAttr(AL, 0, Str, ))
+return;
+
+  // Semantic checks for a function with the 'interrupt' attribute:
+  // - Must be a function.
+  // - Must have no parameters.
+  // - Must have the 'void' return type.
+  // - The attribute itself must either have no argument or one of the
+  //   valid interrupt types, see [RISCVInterruptDocs].
+
+  if (D->getFunctionType() == nullptr) {
+S.Diag(D->getLocation(), diag::warn_attribute_wrong_decl_type)
+  << "'interrupt'" << ExpectedFunction;
+return;
+  }
+
+  if (!hasFunctionProto(D)) {
+S.Diag(D->getLocation(), diag::warn_riscv_interrupt_attribute) << 2;
+return;
+  }
+
+  if (getFunctionOrMethodNumParams(D) != 0) {
+

[PATCH] D48412: [RISCV] Add support for interrupt attribute

[Ana Pazos via Phabricator via cfe-commits]

apazos added inline comments.



Comment at: test/Sema/riscv-interrupt-attr.c:23
+  // expected-note 
{{repeated RISC-V 'interrupt' attribute is here}}
+__attribute__((interrupt("user"))) void foo8() {}
+__attribute__((interrupt("supervisor"))) void foo9() {}

aaron.ballman wrote:
> apazos wrote:
> > aaron.ballman wrote:
> > > apazos wrote:
> > > > aaron.ballman wrote:
> > > > > aaron.ballman wrote:
> > > > > > Do you intend for functions without a prototype to be accepted? 
> > > > > > foo8() can be passed an arbitrary number of arguments, which is a 
> > > > > > bit different than what I thought you wanted the semantic check to 
> > > > > > be.
> > > > > This question remains outstanding.
> > > > The checks are validating both function definitions and function 
> > > > prototypes like these:
> > > > _attribute__((interrupt)) void foo1() {} 
> > > > __attribute__((interrupt)) void foo(void);
> > > > Not sure what the confusion is.
> > > Ah, now I see where the confusion is.
> > > 
> > > In C, an empty parameter list declares a function without a prototype; 
> > > functions without prototypes can accept any number of arguments. To 
> > > declare a function that accepts no arguments, you must have a prototype 
> > > for the function and the parameter list is void. In C++, all functions 
> > > are prototyped and an empty parameter list is equivalent to a parameter 
> > > list of void. The word "prototype" doesn't mean "forward declaration". 
> > > e.g.,
> > > ```
> > > // C code
> > > void foo1(); // Declaration; no prototype; accepts any number of 
> > > arguments.
> > > void foo2() {} // Definition; no prototype; accepts any number of 
> > > arguments.
> > > void foo3(void); // Declaration; prototype; accepts no arguments.
> > > void foo4(void) {} // Definition; prototype; accepts no arguments.
> > > 
> > > foo2(1, 2, 3); // ok
> > > foo4(1, 2, 3); // error
> > > ```
> > > Because a function without a prototype can accept any number of 
> > > arguments, I think you want to diagnose such a function signature.
> > Thanks for clarifying. 
> > 
> > I checked GCC behavior and it is less strict. For the example below, it 
> > silently accepts the interrupt attribute.
> > 
> > extern int foo2();
> > __attribute__((interrupt)) void foo();
> > void foo() {
> >   foo2();
> > }
> > 
> > while in LLVM we would be rejecting with the message: 
> > RISC-V 'interrupt' attribute only applies to functions that have no 
> > parameters. 
> > 
> > I find the reuse of the message confusing. 
> > 
> > If we want stricter rule then we probably also need a specific message for 
> > the  missing prototype.
> > 
> > I checked GCC behavior and it is less strict. For the example below, it 
> > silently accepts the interrupt attribute.
> 
> Does it drop the attribute?
> 
> > If we want stricter rule then we probably also need a specific message for 
> > the missing prototype.
> 
> If GCC silently drops the attribute in this case then we definitely want a 
> more strict rule. We already have a good diagnostic for this: 
> `warn_attribute_wrong_decl_type` with the expected type diagnostic index 
> being `ExpectedFunctionWithProtoType`.
It does not drop, it compiles without warnings and it produces the code that is 
expected when interrupt attribute is set.


https://reviews.llvm.org/D48412



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

[PATCH] D48412: [RISCV] Add support for interrupt attribute

[Ana Pazos via Phabricator via cfe-commits]

apazos added inline comments.



Comment at: test/Sema/riscv-interrupt-attr.c:23
+  // expected-note 
{{repeated RISC-V 'interrupt' attribute is here}}
+__attribute__((interrupt("user"))) void foo8() {}
+__attribute__((interrupt("supervisor"))) void foo9() {}

aaron.ballman wrote:
> apazos wrote:
> > aaron.ballman wrote:
> > > aaron.ballman wrote:
> > > > Do you intend for functions without a prototype to be accepted? foo8() 
> > > > can be passed an arbitrary number of arguments, which is a bit 
> > > > different than what I thought you wanted the semantic check to be.
> > > This question remains outstanding.
> > The checks are validating both function definitions and function prototypes 
> > like these:
> > _attribute__((interrupt)) void foo1() {} 
> > __attribute__((interrupt)) void foo(void);
> > Not sure what the confusion is.
> Ah, now I see where the confusion is.
> 
> In C, an empty parameter list declares a function without a prototype; 
> functions without prototypes can accept any number of arguments. To declare a 
> function that accepts no arguments, you must have a prototype for the 
> function and the parameter list is void. In C++, all functions are prototyped 
> and an empty parameter list is equivalent to a parameter list of void. The 
> word "prototype" doesn't mean "forward declaration". e.g.,
> ```
> // C code
> void foo1(); // Declaration; no prototype; accepts any number of arguments.
> void foo2() {} // Definition; no prototype; accepts any number of arguments.
> void foo3(void); // Declaration; prototype; accepts no arguments.
> void foo4(void) {} // Definition; prototype; accepts no arguments.
> 
> foo2(1, 2, 3); // ok
> foo4(1, 2, 3); // error
> ```
> Because a function without a prototype can accept any number of arguments, I 
> think you want to diagnose such a function signature.
Thanks for clarifying. 

I checked GCC behavior and it is less strict. For the example below, it 
silently accepts the interrupt attribute.

extern int foo2();
__attribute__((interrupt)) void foo();
void foo() {
  foo2();
}

while in LLVM we would be rejecting with the message: 
RISC-V 'interrupt' attribute only applies to functions that have no parameters. 

I find the reuse of the message confusing. 

If we want stricter rule then we probably also need a specific message for the  
missing prototype.



https://reviews.llvm.org/D48412



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

[PATCH] D48412: [RISCV] Add support for interrupt attribute

[Ana Pazos via Phabricator via cfe-commits]

apazos updated this revision to Diff 153544.

https://reviews.llvm.org/D48412

Files:
  include/clang/Basic/Attr.td
  include/clang/Basic/AttrDocs.td
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/CodeGen/TargetInfo.cpp
  lib/Sema/SemaDeclAttr.cpp
  test/Sema/riscv-interrupt-attr.c
  test/Sema/riscv-interrupt-attr.cpp

Index: test/Sema/riscv-interrupt-attr.cpp
===
--- /dev/null
+++ test/Sema/riscv-interrupt-attr.cpp
@@ -0,0 +1,7 @@
+// RUN: %clang_cc1 %s -triple riscv32-unknown-elf -verify -fsyntax-only
+// RUN: %clang_cc1 %s -triple riscv64-unknown-elf -verify -fsyntax-only
+
+[[gnu::interrupt]] [[gnu::interrupt]] void foo1() {} // expected-warning {{repeated RISC-V 'interrupt' attribute}} \
+ // expected-note {{repeated RISC-V 'interrupt' attribute is here}}
+[[gnu::interrupt]] void foo2() {}
+
Index: test/Sema/riscv-interrupt-attr.c
===
--- /dev/null
+++ test/Sema/riscv-interrupt-attr.c
@@ -0,0 +1,34 @@
+// RUN: %clang_cc1 %s -triple riscv32-unknown-elf -verify -fsyntax-only
+// RUN: %clang_cc1 %s -triple riscv64-unknown-elf -verify -fsyntax-only
+
+struct a { int b; };
+
+struct a test __attribute__((interrupt)); // expected-warning {{'interrupt' attribute only applies to functions}}
+
+__attribute__((interrupt("USER"))) void foo1() {} // expected-warning {{'interrupt' attribute argument not supported: USER}}
+
+__attribute__((interrupt("user", 1))) void foo2() {} // expected-error {{'interrupt' attribute takes no more than 1 argument}}
+
+__attribute__((interrupt)) int foo3() {return 0;} // expected-warning {{RISC-V 'interrupt' attribute only applies to functions that have a 'void' return type}}
+
+__attribute__((interrupt())) int foo4(void) {} // expected-warning {{RISC-V 'interrupt' attribute only applies to functions that have a 'void' return type}}
+
+__attribute__((interrupt())) void foo5(int a) {} // expected-warning {{RISC-V 'interrupt' attribute only applies to functions that have no parameters}}
+
+__attribute__((interrupt("user"), interrupt("supervisor"))) void foo6() {} // expected-warning {{repeated RISC-V 'interrupt' attribute}} \
+// expected-note {{repeated RISC-V 'interrupt' attribute is here}}
+
+__attribute__((interrupt, interrupt)) void foo7() {} // expected-warning {{repeated RISC-V 'interrupt' attribute}} \
+ // expected-note {{repeated RISC-V 'interrupt' attribute is here}}
+__attribute__((interrupt(""))) void foo8() {} // expected-warning {{'interrupt' attribute argument not supported}}
+
+__attribute__((interrupt("user"))) void foo9();
+__attribute__((interrupt("supervisor"))) void foo9();
+__attribute__((interrupt("machine"))) void foo9();
+
+__attribute__((interrupt("user"))) void foo10() {}
+__attribute__((interrupt("supervisor"))) void foo11() {}
+__attribute__((interrupt("machine"))) void foo12() {}
+__attribute__((interrupt())) void foo13() {}
+__attribute__((interrupt)) void foo14() {}
+
Index: lib/Sema/SemaDeclAttr.cpp
===
--- lib/Sema/SemaDeclAttr.cpp
+++ lib/Sema/SemaDeclAttr.cpp
@@ -5266,6 +5266,63 @@
   handleSimpleAttribute(S, D, AL);
 }
 
+static void handleRISCVInterruptAttr(Sema , Decl *D,
+ const AttributeList ) {
+  // Warn about repeated attributes.
+  if (const auto *A = D->getAttr()) {
+S.Diag(AL.getRange().getBegin(),
+  diag::warn_riscv_repeated_interrupt_attribute);
+S.Diag(A->getLocation(), diag::note_riscv_repeated_interrupt_attribute);
+return;
+  }
+
+  // Check the attribute argument. Argument is optional.
+  if (!checkAttributeAtMostNumArgs(S, AL, 1))
+return;
+
+  StringRef Str;
+  SourceLocation ArgLoc;
+
+  // 'machine'is the default interrupt mode.
+  if (AL.getNumArgs() == 0)
+Str = "machine";
+  else if (!S.checkStringLiteralArgumentAttr(AL, 0, Str, ))
+return;
+
+  // Semantic checks for a function with the 'interrupt' attribute:
+  // - Must be a function.
+  // - Must have no parameters.
+  // - Must have the 'void' return type.
+  // - The attribute itself must either have no argument or one of the
+  //   valid interrupt types, see [RISCVInterruptDocs].
+
+  if (D->getFunctionType() == nullptr) {
+S.Diag(D->getLocation(), diag::warn_attribute_wrong_decl_type)
+  << "'interrupt'" << ExpectedFunction;
+return;
+  }
+
+  if (hasFunctionProto(D) && getFunctionOrMethodNumParams(D) != 0) {
+S.Diag(D->getLocation(), diag::warn_riscv_interrupt_attribute) << 0;
+return;
+  }
+
+  if (!getFunctionOrMethodResultType(D)->isVoidType()) {
+S.Diag(D->getLocation(), diag::warn_riscv_interrupt_attribute) << 1;
+return;
+  }
+
+  RISCVInterruptAttr::InterruptType Kind;
+  if 

[PATCH] D48412: [RISCV] Add support for interrupt attribute

[Ana Pazos via Phabricator via cfe-commits]

apazos added inline comments.



Comment at: lib/Sema/SemaDeclAttr.cpp:5305
+
+  if (hasFunctionProto(D) && getFunctionOrMethodNumParams(D) != 0) {
+S.Diag(D->getLocation(), diag::warn_riscv_interrupt_attribute) << 0;

aaron.ballman wrote:
> I would have assumed this would be: `!hasFunctionProto(D) || 
> getFunctionOrMethodNumParams(D) != 0`, but it depends on whether you want to 
> support K C functions.
hasFunctionProto also returns true for a function definition like this one 
 __attribute__((interrupt)) void foo1(int) {}.



Comment at: lib/Sema/SemaDeclAttr.cpp:5301
+
+  if (!isFunctionOrMethod(D)) {
+S.Diag(D->getLocation(), diag::warn_attribute_wrong_decl_type)

aaron.ballman wrote:
> apazos wrote:
> > aaron.ballman wrote:
> > > apazos wrote:
> > > > aaron.ballman wrote:
> > > > > I don't think you need to perform this check -- I believe it's 
> > > > > handled automatically (because you don't have custom parsing enabled).
> > > > I think need it. Will double check in the test.
> > > See `handleCommonAttributeFeatures()` -- it calls 
> > > `diagnoseAppertainsTo()` which handles this for you. As it is, your check 
> > > here does not match the subject list on the attribute. The declarative 
> > > bit says it only appertains to a function and this check is for a 
> > > function or Obj-C method.
> > > 
> > > Which brings up my next question: should this appertain to an ObjC method?
> > It looks like handleCommonAttributeFeatures should take care of the check, 
> > but I do not see it happening, it returns true in 
> > AL.diagnoseAppertainsTo(S, D) even when we have 
> > struct a test __attribute__((interrupt));
> > 
> > I will remove the Subjects in Attr.td and keep the checks as they are in 
> > handleRISCVInterruptAttr.
> > 
> > Several other targets do the same thing, they are reusing the helper 
> > functions that apply to both Function or Method.  We would have to create 
> > helper functions just for function types.
> Ah, the reason is because the parsed attributes that share a `ParseKind` can 
> have different subject lists, so there's no way to do the semantic checking 
> at that point -- we don't know which semantic attribute to check the subjects 
> against until later.
> 
> Please put the `Subjects` list back in to Attr.td; it's still useful 
> declarative information and I may solve this problem someday in the future.
> 
> I am not tied to whether the attribute appertains to a function and an obj-c 
> method as that depends on the attribute in question, but the code as it 
> stands is wrong. It checks whether the declaration is a function or a method 
> and then tells the user the attribute can only appertain to a function and 
> not a method. Which is correct?
Sure I can add Subjects back in.
I will remove the helper function and use the simple check D->getFunctionType() 
== nullptr.



Comment at: test/Sema/riscv-interrupt-attr.c:18
+
+__attribute__((interrupt("user"), interrupt("supervisor"))) void foo6() { } // 
expected-warning {{repeated RISC-V 'interrupt' attribute}} \
+// 
expected-note {{repeated RISC-V 'interrupt' attribute is here}}

aaron.ballman wrote:
> apazos wrote:
> > aaron.ballman wrote:
> > > apazos wrote:
> > > > aaron.ballman wrote:
> > > > > You should also add tests for:
> > > > > ```
> > > > > __attribute__((interrupt("user"))) void f(void);
> > > > > __attribute__((interrupt("machine"))) void f(void);
> > > > > 
> > > > > void f(void) { }
> > > > > 
> > > > > [[gnu::interrupt("user") gnu::interrupt("machine")]] void g() {}
> > > > > 
> > > > > [[gnu::interrupt("user")]] [[gnu::interrupt("machine")]] void h() {}
> > > > > ```
> > > > For this test case tt seems LLVM honors the last setting, "machine".
> > > > But gcc is honoring the first.
> > > > I think the last setting should prevail. Will check with GCC folks.
> > > Do all of these cases get diagnosed as being a repeated interrupt 
> > > attribute? Should add them as test cases.
> > The warning for repeated attribute is when it occurs more than once in the 
> > same declaration. If you have repeated declarations, the last one prevails.
> Please document this in AttrDocs.td.
Sure, I can add that info to the description.



Comment at: test/Sema/riscv-interrupt-attr.c:23
+  // expected-note 
{{repeated RISC-V 'interrupt' attribute is here}}
+__attribute__((interrupt("user"))) void foo8() {}
+__attribute__((interrupt("supervisor"))) void foo9() {}

aaron.ballman wrote:
> aaron.ballman wrote:
> > Do you intend for functions without a prototype to be accepted? foo8() can 
> > be passed an arbitrary number of arguments, which is a bit different than 
> > what I thought you wanted the semantic check to be.
> This question remains outstanding.
The checks are 

[PATCH] D48412: [RISCV] Add support for interrupt attribute

[Ana Pazos via Phabricator via cfe-commits]

apazos updated this revision to Diff 153419.
apazos added a comment.

Updated tests and removed Subjects from Attr.td


https://reviews.llvm.org/D48412

Files:
  include/clang/Basic/Attr.td
  include/clang/Basic/AttrDocs.td
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/CodeGen/TargetInfo.cpp
  lib/Sema/SemaDeclAttr.cpp
  test/Sema/riscv-interrupt-attr.c
  test/Sema/riscv-interrupt-attr.cpp

Index: test/Sema/riscv-interrupt-attr.cpp
===
--- /dev/null
+++ test/Sema/riscv-interrupt-attr.cpp
@@ -0,0 +1,7 @@
+// RUN: %clang_cc1 %s -triple riscv32-unknown-elf -verify -fsyntax-only
+// RUN: %clang_cc1 %s -triple riscv64-unknown-elf -verify -fsyntax-only
+
+[[gnu::interrupt]] [[gnu::interrupt]] void foo1() {} // expected-warning {{repeated RISC-V 'interrupt' attribute}} \
+ // expected-note {{repeated RISC-V 'interrupt' attribute is here}}
+[[gnu::interrupt]] void foo2() {}
+
Index: test/Sema/riscv-interrupt-attr.c
===
--- /dev/null
+++ test/Sema/riscv-interrupt-attr.c
@@ -0,0 +1,34 @@
+// RUN: %clang_cc1 %s -triple riscv32-unknown-elf -verify -fsyntax-only
+// RUN: %clang_cc1 %s -triple riscv64-unknown-elf -verify -fsyntax-only
+
+struct a { int b; };
+
+struct a test __attribute__((interrupt)); // expected-warning {{'interrupt' attribute only applies to functions}}
+
+__attribute__((interrupt("USER"))) void foo1() {} // expected-warning {{'interrupt' attribute argument not supported: USER}}
+
+__attribute__((interrupt("user", 1))) void foo2() {} // expected-error {{'interrupt' attribute takes no more than 1 argument}}
+
+__attribute__((interrupt)) int foo3() {return 0;} // expected-warning {{RISC-V 'interrupt' attribute only applies to functions that have a 'void' return type}}
+
+__attribute__((interrupt())) int foo4(void) {} // expected-warning {{RISC-V 'interrupt' attribute only applies to functions that have a 'void' return type}}
+
+__attribute__((interrupt())) void foo5(int a) {} // expected-warning {{RISC-V 'interrupt' attribute only applies to functions that have no parameters}}
+
+__attribute__((interrupt("user"), interrupt("supervisor"))) void foo6() {} // expected-warning {{repeated RISC-V 'interrupt' attribute}} \
+// expected-note {{repeated RISC-V 'interrupt' attribute is here}}
+
+__attribute__((interrupt, interrupt)) void foo7() {} // expected-warning {{repeated RISC-V 'interrupt' attribute}} \
+ // expected-note {{repeated RISC-V 'interrupt' attribute is here}}
+__attribute__((interrupt(""))) void foo8() {} // expected-warning {{'interrupt' attribute argument not supported}}
+
+__attribute__((interrupt("user"))) void foo9();
+__attribute__((interrupt("supervisor"))) void foo9();
+__attribute__((interrupt("machine"))) void foo9();
+
+__attribute__((interrupt("user"))) void foo10() {}
+__attribute__((interrupt("supervisor"))) void foo11() {}
+__attribute__((interrupt("machine"))) void foo12() {}
+__attribute__((interrupt())) void foo13() {}
+__attribute__((interrupt)) void foo14() {}
+
Index: lib/Sema/SemaDeclAttr.cpp
===
--- lib/Sema/SemaDeclAttr.cpp
+++ lib/Sema/SemaDeclAttr.cpp
@@ -5266,6 +5266,63 @@
   handleSimpleAttribute(S, D, AL);
 }
 
+static void handleRISCVInterruptAttr(Sema , Decl *D,
+ const AttributeList ) {
+  // Warn about repeated attributes.
+  if (const auto *A = D->getAttr()) {
+S.Diag(AL.getRange().getBegin(),
+  diag::warn_riscv_repeated_interrupt_attribute);
+S.Diag(A->getLocation(), diag::note_riscv_repeated_interrupt_attribute);
+return;
+  }
+
+  // Check the attribute argument. Argument is optional.
+  if (!checkAttributeAtMostNumArgs(S, AL, 1))
+return;
+
+  StringRef Str;
+  SourceLocation ArgLoc;
+
+  // 'machine'is the default interrupt mode.
+  if (AL.getNumArgs() == 0)
+Str = "machine";
+  else if (!S.checkStringLiteralArgumentAttr(AL, 0, Str, ))
+return;
+
+  // Semantic checks for a function with the 'interrupt' attribute:
+  // - Must be a function.
+  // - Must have no parameters.
+  // - Must have the 'void' return type.
+  // - The attribute itself must either have no argument or one of the
+  //   valid interrupt types, see [RISCVInterruptDocs].
+
+  if (!isFunctionOrMethod(D)) {
+S.Diag(D->getLocation(), diag::warn_attribute_wrong_decl_type)
+  << "'interrupt'" << ExpectedFunction;
+return;
+  }
+
+  if (hasFunctionProto(D) && getFunctionOrMethodNumParams(D) != 0) {
+S.Diag(D->getLocation(), diag::warn_riscv_interrupt_attribute) << 0;
+return;
+  }
+
+  if (!getFunctionOrMethodResultType(D)->isVoidType()) {
+S.Diag(D->getLocation(), diag::warn_riscv_interrupt_attribute) << 1;
+return;
+  }
+
+  

[PATCH] D48412: [RISCV] Add support for interrupt attribute

[Ana Pazos via Phabricator via cfe-commits]

apazos added inline comments.



Comment at: lib/Sema/SemaDeclAttr.cpp:5301
+
+  if (!isFunctionOrMethod(D)) {
+S.Diag(D->getLocation(), diag::warn_attribute_wrong_decl_type)

aaron.ballman wrote:
> apazos wrote:
> > aaron.ballman wrote:
> > > I don't think you need to perform this check -- I believe it's handled 
> > > automatically (because you don't have custom parsing enabled).
> > I think need it. Will double check in the test.
> See `handleCommonAttributeFeatures()` -- it calls `diagnoseAppertainsTo()` 
> which handles this for you. As it is, your check here does not match the 
> subject list on the attribute. The declarative bit says it only appertains to 
> a function and this check is for a function or Obj-C method.
> 
> Which brings up my next question: should this appertain to an ObjC method?
It looks like handleCommonAttributeFeatures should take care of the check, but 
I do not see it happening, it returns true in AL.diagnoseAppertainsTo(S, D) 
even when we have 
struct a test __attribute__((interrupt));

I will remove the Subjects in Attr.td and keep the checks as they are in 
handleRISCVInterruptAttr.

Several other targets do the same thing, they are reusing the helper functions 
that apply to both Function or Method.  We would have to create helper 
functions just for function types.



Comment at: test/Sema/riscv-interrupt-attr.c:18
+
+__attribute__((interrupt("user"), interrupt("supervisor"))) void foo6() { } // 
expected-warning {{repeated RISC-V 'interrupt' attribute}} \
+// 
expected-note {{repeated RISC-V 'interrupt' attribute is here}}

aaron.ballman wrote:
> apazos wrote:
> > aaron.ballman wrote:
> > > You should also add tests for:
> > > ```
> > > __attribute__((interrupt("user"))) void f(void);
> > > __attribute__((interrupt("machine"))) void f(void);
> > > 
> > > void f(void) { }
> > > 
> > > [[gnu::interrupt("user") gnu::interrupt("machine")]] void g() {}
> > > 
> > > [[gnu::interrupt("user")]] [[gnu::interrupt("machine")]] void h() {}
> > > ```
> > For this test case tt seems LLVM honors the last setting, "machine".
> > But gcc is honoring the first.
> > I think the last setting should prevail. Will check with GCC folks.
> Do all of these cases get diagnosed as being a repeated interrupt attribute? 
> Should add them as test cases.
The warning for repeated attribute is when it occurs more than once in the same 
declaration. If you have repeated declarations, the last one prevails.


https://reviews.llvm.org/D48412



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

[PATCH] D48412: [RISCV] Add support for interrupt attribute

[Ana Pazos via Phabricator via cfe-commits]

apazos updated this revision to Diff 152560.
apazos added a comment.

Addressed review comments.


https://reviews.llvm.org/D48412

Files:
  include/clang/Basic/Attr.td
  include/clang/Basic/AttrDocs.td
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/CodeGen/TargetInfo.cpp
  lib/Sema/SemaDeclAttr.cpp
  test/Sema/riscv-interrupt-attr.c

Index: test/Sema/riscv-interrupt-attr.c
===
--- /dev/null
+++ test/Sema/riscv-interrupt-attr.c
@@ -0,0 +1,30 @@
+// RUN: %clang_cc1 %s -triple riscv32-unknown-elf -verify -fsyntax-only
+// RUN: %clang_cc1 %s -triple riscv64-unknown-elf -verify -fsyntax-only
+
+struct a { int b; };
+
+struct a test __attribute__((interrupt)); // expected-warning {{'interrupt' attribute only applies to functions}}
+
+__attribute__((interrupt("USER"))) void foo1() {} // expected-warning {{'interrupt' attribute argument not supported: USER}}
+
+__attribute__((interrupt("user", 1))) void foo2() {} // expected-error {{'interrupt' attribute takes no more than 1 argument}}
+
+__attribute__((interrupt)) int foo3() {return 0;} // expected-warning {{RISC-V 'interrupt' attribute only applies to functions that have a 'void' return type}}
+
+__attribute__((interrupt())) int foo4(void) {} // expected-warning {{RISC-V 'interrupt' attribute only applies to functions that have a 'void' return type}}
+
+__attribute__((interrupt())) void foo5(int a) {} // expected-warning {{RISC-V 'interrupt' attribute only applies to functions that have no parameters}}
+
+__attribute__((interrupt("user"), interrupt("supervisor"))) void foo6() {} // expected-warning {{repeated RISC-V 'interrupt' attribute}} \
+// expected-note {{repeated RISC-V 'interrupt' attribute is here}}
+
+__attribute__((interrupt, interrupt)) void foo7() {} // expected-warning {{repeated RISC-V 'interrupt' attribute}} \
+ // expected-note {{repeated RISC-V 'interrupt' attribute is here}}
+__attribute__((interrupt(""))) void foo11() {} // expected-warning {{'interrupt' attribute argument not supported}}
+
+__attribute__((interrupt("user"))) void foo8() {}
+__attribute__((interrupt("supervisor"))) void foo9() {}
+__attribute__((interrupt("machine"))) void foo10() {}
+__attribute__((interrupt())) void foo12() {}
+__attribute__((interrupt)) void foo13() {}
+
Index: lib/Sema/SemaDeclAttr.cpp
===
--- lib/Sema/SemaDeclAttr.cpp
+++ lib/Sema/SemaDeclAttr.cpp
@@ -5266,6 +5266,63 @@
   handleSimpleAttribute(S, D, AL);
 }
 
+static void handleRISCVInterruptAttr(Sema , Decl *D,
+ const AttributeList ) {
+  // Warn about repeated attributes.
+  if (const auto *A = D->getAttr()) {
+S.Diag(AL.getRange().getBegin(),
+  diag::warn_riscv_repeated_interrupt_attribute);
+S.Diag(A->getLocation(), diag::note_riscv_repeated_interrupt_attribute);
+return;
+  }
+
+  // Check the attribute argument. Argument is optional.
+  if (!checkAttributeAtMostNumArgs(S, AL, 1))
+return;
+
+  StringRef Str;
+  SourceLocation ArgLoc;
+
+  // 'machine'is the default interrupt mode.
+  if (AL.getNumArgs() == 0)
+Str = "machine";
+  else if (!S.checkStringLiteralArgumentAttr(AL, 0, Str, ))
+return;
+
+  // Semantic checks for a function with the 'interrupt' attribute:
+  // - Must be a function.
+  // - Must have no parameters.
+  // - Must have the 'void' return type.
+  // - The attribute itself must either have no argument or one of the
+  //   valid interrupt types, see [RISCVInterruptDocs].
+
+  if (!isFunctionOrMethod(D)) {
+S.Diag(D->getLocation(), diag::warn_attribute_wrong_decl_type)
+  << "'interrupt'" << ExpectedFunction;
+return;
+  }
+
+  if (hasFunctionProto(D) && getFunctionOrMethodNumParams(D) != 0) {
+S.Diag(D->getLocation(), diag::warn_riscv_interrupt_attribute) << 0;
+return;
+  }
+
+  if (!getFunctionOrMethodResultType(D)->isVoidType()) {
+S.Diag(D->getLocation(), diag::warn_riscv_interrupt_attribute) << 1;
+return;
+  }
+
+  RISCVInterruptAttr::InterruptType Kind;
+  if (!RISCVInterruptAttr::ConvertStrToInterruptType(Str, Kind)) {
+S.Diag(AL.getLoc(), diag::warn_attribute_type_not_supported)
+  << AL.getName() << Str << ArgLoc;
+return;
+  }
+
+  D->addAttr(::new (S.Context) RISCVInterruptAttr(
+AL.getLoc(), S.Context, Kind, AL.getAttributeSpellingListIndex()));
+}
+
 static void handleInterruptAttr(Sema , Decl *D, const AttributeList ) {
   // Dispatch the interrupt attribute based on the current target.
   switch (S.Context.getTargetInfo().getTriple().getArch()) {
@@ -5283,6 +5340,10 @@
   case llvm::Triple::avr:
 handleAVRInterruptAttr(S, D, AL);
 break;
+  case llvm::Triple::riscv32:
+  case llvm::Triple::riscv64:
+handleRISCVInterruptAttr(S, D, AL);
+break;
   default:
 

[PATCH] D48412: [RISCV] Add support for interrupt attribute

[Ana Pazos via Phabricator via cfe-commits]

apazos added inline comments.



Comment at: lib/CodeGen/TargetInfo.cpp:8966
+
+const RISCVInterruptAttr *Attr = FD->getAttr();
+if (!Attr)

aaron.ballman wrote:
> You can use `const auto *` here instead of repeating the type.
Thanks Aaron, will do the cleanup.



Comment at: lib/Sema/SemaDeclAttr.cpp:5280
+  // Check the attribute arguments.
+  if (AL.getNumArgs() > 1) {
+S.Diag(AL.getLoc(), diag::err_attribute_too_many_arguments)

aaron.ballman wrote:
> Please call `checkAttributeNumArgs()` instead; the error you're using is 
> incorrect (it's used for variadic parameters where you receive more arguments 
> than you expect).
The argument is optional and at most one argument , I will use 
checkAttributeAtMostNumArgs instead



Comment at: lib/Sema/SemaDeclAttr.cpp:5301
+
+  if (!isFunctionOrMethod(D)) {
+S.Diag(D->getLocation(), diag::warn_attribute_wrong_decl_type)

aaron.ballman wrote:
> I don't think you need to perform this check -- I believe it's handled 
> automatically (because you don't have custom parsing enabled).
I think need it. Will double check in the test.



Comment at: test/Sema/riscv-interrupt-attr.c:18
+
+__attribute__((interrupt("user"), interrupt("supervisor"))) void foo6() { } // 
expected-warning {{repeated RISC-V 'interrupt' attribute}} \
+// 
expected-note {{repeated RISC-V 'interrupt' attribute is here}}

aaron.ballman wrote:
> You should also add tests for:
> ```
> __attribute__((interrupt("user"))) void f(void);
> __attribute__((interrupt("machine"))) void f(void);
> 
> void f(void) { }
> 
> [[gnu::interrupt("user") gnu::interrupt("machine")]] void g() {}
> 
> [[gnu::interrupt("user")]] [[gnu::interrupt("machine")]] void h() {}
> ```
For this test case tt seems LLVM honors the last setting, "machine".
But gcc is honoring the first.
I think the last setting should prevail. Will check with GCC folks.



Comment at: test/Sema/riscv-interrupt-attr.c:26
+__attribute__((interrupt("machine"))) void foo10() {}
+__attribute__((interrupt(""))) void foo11() {}
+__attribute__((interrupt())) void foo12() {}

aaron.ballman wrote:
> I'm a bit surprised that this is not an error -- the argument is provided, so 
> I don't know why this should be treated as acceptable.
Good catch, In include/clang/Basic/Attr.td I mapped "" to the default machine 
mode.
I will change it to align with GCC behavior.


https://reviews.llvm.org/D48412



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

[PATCH] D48412: [RISCV] Support for __attribute__((interrupt))

[Ana Pazos via Phabricator via cfe-commits]

apazos updated this revision to Diff 152416.
apazos added a comment.
Herald added subscribers: brucehoult, MartinMosbeck.

Hi Simon, I have added a warning for repeated interrupt attributes.


https://reviews.llvm.org/D48412

Files:
  include/clang/Basic/Attr.td
  include/clang/Basic/AttrDocs.td
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/CodeGen/TargetInfo.cpp
  lib/Sema/SemaDeclAttr.cpp
  test/Sema/riscv-interrupt-attr.c

Index: test/Sema/riscv-interrupt-attr.c
===
--- /dev/null
+++ test/Sema/riscv-interrupt-attr.c
@@ -0,0 +1,29 @@
+// RUN: %clang_cc1 %s -triple riscv32-unknown-elf -verify -fsyntax-only
+// RUN: %clang_cc1 %s -triple riscv64-unknown-elf -verify -fsyntax-only
+
+struct a { int b; };
+
+struct a test __attribute__((interrupt)); // expected-warning {{'interrupt' attribute only applies to functions}}
+
+__attribute__((interrupt("USER"))) void foo1() {} // expected-warning {{'interrupt' attribute argument not supported: USER}}
+
+__attribute__((interrupt("user", 1))) void foo2() {} // expected-error {{'interrupt' attribute takes no more than 1 argument}}
+
+__attribute__((interrupt)) int foo3() {return 0;} // expected-warning {{RISC-V 'interrupt' attribute only applies to functions that have a 'void' return type}}
+
+__attribute__((interrupt())) int foo4(void) { } // expected-warning {{RISC-V 'interrupt' attribute only applies to functions that have a 'void' return type}}
+
+__attribute__((interrupt())) void foo5(int a) { } // expected-warning {{RISC-V 'interrupt' attribute only applies to functions that have no parameters}}
+
+__attribute__((interrupt("user"), interrupt("supervisor"))) void foo6() { } // expected-warning {{repeated RISC-V 'interrupt' attribute}} \
+// expected-note {{repeated RISC-V 'interrupt' attribute is here}}
+
+__attribute__((interrupt, interrupt)) void foo7() { } // expected-warning {{repeated RISC-V 'interrupt' attribute}} \
+  // expected-note {{repeated RISC-V 'interrupt' attribute is here}}
+__attribute__((interrupt("user"))) void foo8() {}
+__attribute__((interrupt("supervisor"))) void foo9() {}
+__attribute__((interrupt("machine"))) void foo10() {}
+__attribute__((interrupt(""))) void foo11() {}
+__attribute__((interrupt())) void foo12() {}
+__attribute__((interrupt)) void foo13() {}
+
Index: lib/Sema/SemaDeclAttr.cpp
===
--- lib/Sema/SemaDeclAttr.cpp
+++ lib/Sema/SemaDeclAttr.cpp
@@ -5266,6 +5266,65 @@
   handleSimpleAttribute(S, D, AL);
 }
 
+static void handleRISCVInterruptAttr(Sema , Decl *D,
+ const AttributeList ) {
+  // Warn about repeated attributes.
+  if (const auto *A = D->getAttr()) {
+S.Diag(AL.getRange().getBegin(),
+  diag::warn_riscv_repeated_interrupt_attribute);
+S.Diag(A->getLocation(), diag::note_riscv_repeated_interrupt_attribute);
+return;
+  }
+
+  // Check the attribute arguments.
+  if (AL.getNumArgs() > 1) {
+S.Diag(AL.getLoc(), diag::err_attribute_too_many_arguments)
+  << AL.getName() << 1;
+return;
+  }
+
+  StringRef Str;
+  SourceLocation ArgLoc;
+
+  if (AL.getNumArgs() == 0)
+Str = "";
+  else if (!S.checkStringLiteralArgumentAttr(AL, 0, Str, ))
+return;
+
+  // Semantic checks for a function with the 'interrupt' attribute:
+  // - Must be a function.
+  // - Must have no parameters.
+  // - Must have the 'void' return type.
+  // - The attribute itself must either have no argument or one of the
+  //   valid interrupt types, see [RISCVInterruptDocs].
+
+  if (!isFunctionOrMethod(D)) {
+S.Diag(D->getLocation(), diag::warn_attribute_wrong_decl_type)
+  << "'interrupt'" << ExpectedFunction;
+return;
+  }
+
+  if (hasFunctionProto(D) && getFunctionOrMethodNumParams(D) != 0) {
+S.Diag(D->getLocation(), diag::warn_riscv_interrupt_attribute) << 0;
+return;
+  }
+
+  if (!getFunctionOrMethodResultType(D)->isVoidType()) {
+S.Diag(D->getLocation(), diag::warn_riscv_interrupt_attribute) << 1;
+return;
+  }
+
+  RISCVInterruptAttr::InterruptType Kind;
+  if (!RISCVInterruptAttr::ConvertStrToInterruptType(Str, Kind)) {
+S.Diag(AL.getLoc(), diag::warn_attribute_type_not_supported)
+  << AL.getName() << Str << ArgLoc;
+return;
+  }
+
+  D->addAttr(::new (S.Context) RISCVInterruptAttr(
+AL.getLoc(), S.Context, Kind, AL.getAttributeSpellingListIndex()));
+}
+
 static void handleInterruptAttr(Sema , Decl *D, const AttributeList ) {
   // Dispatch the interrupt attribute based on the current target.
   switch (S.Context.getTargetInfo().getTriple().getArch()) {
@@ -5283,6 +5342,10 @@
   case llvm::Triple::avr:
 handleAVRInterruptAttr(S, D, AL);
 break;
+  case llvm::Triple::riscv32:
+  case llvm::Triple::riscv64:
+handleRISCVInterruptAttr(S, D, AL);
+break;
  

[PATCH] D48412: [RISCV] Support for __attribute__((interrupt))

[Ana Pazos via Phabricator via cfe-commits]

apazos created this revision.
apazos added a reviewer: asb.
Herald added subscribers: rogfer01, mgrang, edward-jones, zzheng, shiva0217, 
kito-cheng, niosHD, sabuasal, simoncook, johnrusso, rbar.

Clang supports the GNU style ``__attribute__((interrupt))`` attribute  on RISCV 
targets.
Permissible values for this parameter are user, supervisor, and machine.
If there is no parameter, then it defaults to machine.
Reference: https://gcc.gnu.org/onlinedocs/gcc/RISC-V-Function-Attributes.html


https://reviews.llvm.org/D48412

Files:
  include/clang/Basic/Attr.td
  include/clang/Basic/AttrDocs.td
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/CodeGen/TargetInfo.cpp
  lib/Sema/SemaDeclAttr.cpp
  test/Sema/riscv-interrupt-attr.c

Index: test/Sema/riscv-interrupt-attr.c
===
--- /dev/null
+++ test/Sema/riscv-interrupt-attr.c
@@ -0,0 +1,24 @@
+// RUN: %clang_cc1 %s -triple riscv32-unknown-elf -verify -fsyntax-only
+// RUN: %clang_cc1 %s -triple riscv64-unknown-elf -verify -fsyntax-only
+
+struct a { int b; };
+
+struct a test __attribute__((interrupt)); // expected-warning {{'interrupt' attribute only applies to functions}}
+
+__attribute__((interrupt("USER"))) void foo1() {} // expected-warning {{'interrupt' attribute argument not supported: USER}}
+
+__attribute__((interrupt("user", 1))) void foo2() {} // expected-error {{'interrupt' attribute takes no more than 1 argument}}
+
+__attribute__((interrupt)) int foo3() {return 0;} // expected-warning {{RISC-V 'interrupt' attribute only applies to functions that have a 'void' return type}}
+
+__attribute__((interrupt())) int foo4(void) { } // expected-warning {{RISC-V 'interrupt' attribute only applies to functions that have a 'void' return type}}
+
+__attribute__((interrupt())) void foo5(int a) { } // expected-warning {{RISC-V 'interrupt' attribute only applies to functions that have no parameters}}
+
+__attribute__((interrupt("user"))) void foo6() {}
+__attribute__((interrupt("supervisor"))) void foo7() {}
+__attribute__((interrupt("machine"))) void foo8() {}
+__attribute__((interrupt(""))) void foo9() {}
+__attribute__((interrupt())) void foo10() {}
+__attribute__((interrupt)) void foo11() {}
+
Index: lib/Sema/SemaDeclAttr.cpp
===
--- lib/Sema/SemaDeclAttr.cpp
+++ lib/Sema/SemaDeclAttr.cpp
@@ -5266,6 +5266,60 @@
   handleSimpleAttribute(S, D, AL);
 }
 
+static void handleRISCVInterruptAttr(Sema , Decl *D,
+ const AttributeList ) {
+  // Check the attribute arguments.
+  if (AL.getNumArgs() > 1) {
+S.Diag(AL.getLoc(), diag::err_attribute_too_many_arguments)
+  << AL.getName() << 1;
+return;
+  }
+
+  StringRef Str;
+  SourceLocation ArgLoc;
+
+  if (AL.getNumArgs() == 0)
+Str = "";
+  else if (!S.checkStringLiteralArgumentAttr(AL, 0, Str, ))
+return;
+
+  // Semantic checks for a function with the 'interrupt' attribute:
+  // - Must be a function.
+  // - Must have no parameters.
+  // - Must have the 'void' return type.
+  // - The attribute itself must either have no argument or one of the
+  //   valid interrupt types, see [RISCVInterruptDocs].
+
+  if (!isFunctionOrMethod(D)) {
+S.Diag(D->getLocation(), diag::warn_attribute_wrong_decl_type)
+<< "'interrupt'" << ExpectedFunction;
+return;
+  }
+
+  if (hasFunctionProto(D) && getFunctionOrMethodNumParams(D) != 0) {
+S.Diag(D->getLocation(), diag::warn_riscv_interrupt_attribute)
+<< 0;
+return;
+  }
+
+  if (!getFunctionOrMethodResultType(D)->isVoidType()) {
+S.Diag(D->getLocation(), diag::warn_riscv_interrupt_attribute)
+<< 1;
+return;
+  }
+
+  RISCVInterruptAttr::InterruptType Kind;
+  if (!RISCVInterruptAttr::ConvertStrToInterruptType(Str, Kind)) {
+S.Diag(AL.getLoc(), diag::warn_attribute_type_not_supported)
+  << AL.getName() << Str << ArgLoc;
+return;
+  }
+
+  D->addAttr(::new (S.Context) RISCVInterruptAttr(
+AL.getLoc(), S.Context, Kind, AL.getAttributeSpellingListIndex()));
+
+}
+
 static void handleInterruptAttr(Sema , Decl *D, const AttributeList ) {
   // Dispatch the interrupt attribute based on the current target.
   switch (S.Context.getTargetInfo().getTriple().getArch()) {
@@ -5283,6 +5337,10 @@
   case llvm::Triple::avr:
 handleAVRInterruptAttr(S, D, AL);
 break;
+  case llvm::Triple::riscv32:
+  case llvm::Triple::riscv64:
+handleRISCVInterruptAttr(S, D, AL);
+break;
   default:
 handleARMInterruptAttr(S, D, AL);
 break;
Index: lib/CodeGen/TargetInfo.cpp
===
--- lib/CodeGen/TargetInfo.cpp
+++ lib/CodeGen/TargetInfo.cpp
@@ -8957,6 +8957,27 @@
 public:
   RISCVTargetCodeGenInfo(CodeGen::CodeGenTypes , unsigned XLen)
   : TargetCodeGenInfo(new RISCVABIInfo(CGT, XLen)) {}
+
+  void setTargetAttributes(const Decl *D, llvm::GlobalValue *GV,
+

[PATCH] D39053: [Bitfield] Add more cases to making the bitfield a separate location

[Ana Pazos via Phabricator via cfe-commits]

apazos added a comment.

Thanks for updating the patch, @spetrovic.
Can we have this committed?
This patch has shown to produce code size improvements for a number of targets 
(Mips, X86, ARM, RISC-V).


https://reviews.llvm.org/D39053



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

[PATCH] D45284: [RISCV] More validations on the input value of -march=

[Ana Pazos via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL330880: [RISCV] More validations on the input value of 
-march= (authored by apazos, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D45284?vs=143674=144031#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D45284

Files:
  cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td
  cfe/trunk/lib/Driver/ToolChains/Arch/RISCV.cpp
  cfe/trunk/test/Driver/riscv-arch.c

Index: cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td
===
--- cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td
+++ cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td
@@ -24,6 +24,10 @@
 def err_drv_unknown_language : Error<"language not recognized: '%0'">;
 def err_drv_invalid_arch_name : Error<
   "invalid arch name '%0'">;
+def err_drv_invalid_riscv_arch_name : Error<
+  "invalid arch name '%0', %1">;
+def err_drv_invalid_riscv_ext_arch_name : Error<
+  "invalid arch name '%0', %1 '%2'">;
 def err_drv_cuda_bad_gpu_arch : Error<"Unsupported CUDA gpu architecture: %0">;
 def err_drv_no_cuda_installation : Error<
   "cannot find CUDA installation.  Provide its path via --cuda-path, or pass "
Index: cfe/trunk/test/Driver/riscv-arch.c
===
--- cfe/trunk/test/Driver/riscv-arch.c
+++ cfe/trunk/test/Driver/riscv-arch.c
@@ -1,89 +1,317 @@
-// RUN: %clang -target riscv32-unknown-elf -march=rv32i -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv32-unknown-elf -march=rv32im -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv32-unknown-elf -march=rv32ima -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv32-unknown-elf -march=rv32imaf -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv32-unknown-elf -march=rv32imafd -### %s -fsyntax-only 2>&1 | FileCheck %s
-
-// RUN: %clang -target riscv32-unknown-elf -march=rv32ic -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv32-unknown-elf -march=rv32imc -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv32-unknown-elf -march=rv32imac -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv32-unknown-elf -march=rv32imafc -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv32-unknown-elf -march=rv32imafdc -### %s -fsyntax-only 2>&1 | FileCheck %s
-
-// RUN: %clang -target riscv32-unknown-elf -march=rv32ia -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv32-unknown-elf -march=rv32iaf -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv32-unknown-elf -march=rv32iafd -### %s -fsyntax-only 2>&1 | FileCheck %s
-
-// RUN: %clang -target riscv32-unknown-elf -march=rv32iac -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv32-unknown-elf -march=rv32iafc -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv32-unknown-elf -march=rv32iafdc -### %s -fsyntax-only 2>&1 | FileCheck %s
-
-// RUN: %clang -target riscv32-unknown-elf -march=rv32g -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv32-unknown-elf -march=rv32gc -### %s -fsyntax-only 2>&1 | FileCheck %s
-
-// RUN: %clang -target riscv64-unknown-elf -march=rv64i -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv64-unknown-elf -march=rv64im -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv64-unknown-elf -march=rv64ima -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv64-unknown-elf -march=rv64imaf -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv64-unknown-elf -march=rv64imafd -### %s -fsyntax-only 2>&1 | FileCheck %s
-
-// RUN: %clang -target riscv64-unknown-elf -march=rv64ic -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv64-unknown-elf -march=rv64imc -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv64-unknown-elf -march=rv64imac -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv64-unknown-elf -march=rv64imafc -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv64-unknown-elf -march=rv64imafdc -### %s -fsyntax-only 2>&1 | FileCheck %s
-
-// RUN: %clang -target riscv64-unknown-elf -march=rv64ia -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv64-unknown-elf -march=rv64iaf -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv64-unknown-elf -march=rv64iafd -### %s -fsyntax-only 2>&1 | FileCheck %s
-
-// RUN: %clang -target riscv64-unknown-elf -march=rv64iac -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv64-unknown-elf -march=rv64iafc -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv64-unknown-elf -march=rv64iafdc -### %s -fsyntax-only 2>&1 | FileCheck %s
-
-// RUN: %clang 

[PATCH] D45284: [RISCV] More validations on the input value of -march=

[Ana Pazos via Phabricator via cfe-commits]

apazos updated this revision to Diff 143674.
apazos added a comment.

Hi Alex, the refactoring will be simple and can be done later when we need it, 
all the pieces are already parsed (type, name, major, minor) and are in 
strings,  we will only need to convert to the preferred type (enum, int, etc).
I changed the code to not error out on unsupported extension, allowing all the 
non-standard extensions to be parsed and the format errors reported. Only when 
we try to set target features I then enforce unsupported extension.
This way I could push all my tests.


https://reviews.llvm.org/D45284

Files:
  include/clang/Basic/DiagnosticDriverKinds.td
  lib/Driver/ToolChains/Arch/RISCV.cpp
  test/Driver/riscv-arch.c

Index: test/Driver/riscv-arch.c
===
--- test/Driver/riscv-arch.c
+++ test/Driver/riscv-arch.c
@@ -1,89 +1,317 @@
-// RUN: %clang -target riscv32-unknown-elf -march=rv32i -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv32-unknown-elf -march=rv32im -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv32-unknown-elf -march=rv32ima -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv32-unknown-elf -march=rv32imaf -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv32-unknown-elf -march=rv32imafd -### %s -fsyntax-only 2>&1 | FileCheck %s
-
-// RUN: %clang -target riscv32-unknown-elf -march=rv32ic -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv32-unknown-elf -march=rv32imc -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv32-unknown-elf -march=rv32imac -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv32-unknown-elf -march=rv32imafc -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv32-unknown-elf -march=rv32imafdc -### %s -fsyntax-only 2>&1 | FileCheck %s
-
-// RUN: %clang -target riscv32-unknown-elf -march=rv32ia -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv32-unknown-elf -march=rv32iaf -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv32-unknown-elf -march=rv32iafd -### %s -fsyntax-only 2>&1 | FileCheck %s
-
-// RUN: %clang -target riscv32-unknown-elf -march=rv32iac -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv32-unknown-elf -march=rv32iafc -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv32-unknown-elf -march=rv32iafdc -### %s -fsyntax-only 2>&1 | FileCheck %s
-
-// RUN: %clang -target riscv32-unknown-elf -march=rv32g -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv32-unknown-elf -march=rv32gc -### %s -fsyntax-only 2>&1 | FileCheck %s
-
-// RUN: %clang -target riscv64-unknown-elf -march=rv64i -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv64-unknown-elf -march=rv64im -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv64-unknown-elf -march=rv64ima -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv64-unknown-elf -march=rv64imaf -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv64-unknown-elf -march=rv64imafd -### %s -fsyntax-only 2>&1 | FileCheck %s
-
-// RUN: %clang -target riscv64-unknown-elf -march=rv64ic -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv64-unknown-elf -march=rv64imc -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv64-unknown-elf -march=rv64imac -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv64-unknown-elf -march=rv64imafc -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv64-unknown-elf -march=rv64imafdc -### %s -fsyntax-only 2>&1 | FileCheck %s
-
-// RUN: %clang -target riscv64-unknown-elf -march=rv64ia -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv64-unknown-elf -march=rv64iaf -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv64-unknown-elf -march=rv64iafd -### %s -fsyntax-only 2>&1 | FileCheck %s
-
-// RUN: %clang -target riscv64-unknown-elf -march=rv64iac -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv64-unknown-elf -march=rv64iafc -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv64-unknown-elf -march=rv64iafdc -### %s -fsyntax-only 2>&1 | FileCheck %s
-
-// RUN: %clang -target riscv64-unknown-elf -march=rv64g -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv64-unknown-elf -march=rv64gc -### %s -fsyntax-only 2>&1 | FileCheck %s
+// RUN: %clang -target riscv32-unknown-elf -march=rv32i -### %s \
+// RUN: -fsyntax-only 2>&1 | FileCheck %s
+// RUN: %clang -target riscv32-unknown-elf -march=rv32im -### %s \
+// RUN: -fsyntax-only 2>&1 | FileCheck %s
+// RUN: %clang -target riscv32-unknown-elf -march=rv32ima -### %s \
+// RUN: -fsyntax-only 2>&1 | FileCheck %s
+// RUN: %clang -target riscv32-unknown-elf -march=rv32imaf -### %s \
+// RUN: -fsyntax-only 2>&1 | 

[PATCH] D45284: [RISCV] More validations on the input value of -march=

[Ana Pazos via Phabricator via cfe-commits]

apazos added a comment.

Hi Alex, it seems the table expects these extensions in a canonical order too: 
all x extensions, followed by all s extensions, and then all sx extensions.

I can make the change, no problem. I have also coded other error situations 
described below.

But f I cannot push a test we can enable, because we error out when we find the 
first non-supported extension in the string with unsupported extension message, 
and stop parsing the string.

Any suggestion?

Examples:

clang -target riscv32-unknown-elf -march=rv32ixabc_ -### 
 extension name missing after separator '_'

clang -target riscv32-unknown-elf -march=rv32ixabc_a  -###
invalid extension prefix 'a'

clang -target riscv32-unknown-elf -march=rv32isabc_xdef -###
non-standard user-level extension not given in canonical order 'xdef'

clang -target riscv32-unknown-elf -march=rv32isxabc_sdef -### 
standard supervisor-level extension not given in canonical order 'sdef'

clang -target riscv32-unknown-elf -march=rv32ixabc_xabc -### 
duplicated non-standard user-level extension 'xabc'

clang -target riscv32-unknown-elf -march=rv32ixabc_xdef -###
no parsing error, should be accepted if xabc and xdef are valid extensions

clang -target riscv32-unknown-elf -march=rv32ixabc_sdef_sxghi -### 
no parsing error, should be accepted if xabc sdef sxghi are valid extensions


https://reviews.llvm.org/D45284



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

[PATCH] D45284: [RISCV] More validations on the input value of -march=

[Ana Pazos via Phabricator via cfe-commits]

apazos added a comment.

Addressed the latest review comments.
Added TODOs for validations we cannot do now.


https://reviews.llvm.org/D45284



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

[PATCH] D45284: [RISCV] More validations on the input value of -march=

[Ana Pazos via Phabricator via cfe-commits]

apazos updated this revision to Diff 142716.
apazos edited the summary of this revision.

https://reviews.llvm.org/D45284

Files:
  include/clang/Basic/DiagnosticDriverKinds.td
  lib/Driver/ToolChains/Arch/RISCV.cpp
  test/Driver/riscv-arch.c

Index: test/Driver/riscv-arch.c
===
--- test/Driver/riscv-arch.c
+++ test/Driver/riscv-arch.c
@@ -1,89 +1,262 @@
-// RUN: %clang -target riscv32-unknown-elf -march=rv32i -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv32-unknown-elf -march=rv32im -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv32-unknown-elf -march=rv32ima -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv32-unknown-elf -march=rv32imaf -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv32-unknown-elf -march=rv32imafd -### %s -fsyntax-only 2>&1 | FileCheck %s
-
-// RUN: %clang -target riscv32-unknown-elf -march=rv32ic -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv32-unknown-elf -march=rv32imc -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv32-unknown-elf -march=rv32imac -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv32-unknown-elf -march=rv32imafc -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv32-unknown-elf -march=rv32imafdc -### %s -fsyntax-only 2>&1 | FileCheck %s
-
-// RUN: %clang -target riscv32-unknown-elf -march=rv32ia -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv32-unknown-elf -march=rv32iaf -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv32-unknown-elf -march=rv32iafd -### %s -fsyntax-only 2>&1 | FileCheck %s
-
-// RUN: %clang -target riscv32-unknown-elf -march=rv32iac -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv32-unknown-elf -march=rv32iafc -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv32-unknown-elf -march=rv32iafdc -### %s -fsyntax-only 2>&1 | FileCheck %s
-
-// RUN: %clang -target riscv32-unknown-elf -march=rv32g -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv32-unknown-elf -march=rv32gc -### %s -fsyntax-only 2>&1 | FileCheck %s
-
-// RUN: %clang -target riscv64-unknown-elf -march=rv64i -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv64-unknown-elf -march=rv64im -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv64-unknown-elf -march=rv64ima -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv64-unknown-elf -march=rv64imaf -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv64-unknown-elf -march=rv64imafd -### %s -fsyntax-only 2>&1 | FileCheck %s
-
-// RUN: %clang -target riscv64-unknown-elf -march=rv64ic -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv64-unknown-elf -march=rv64imc -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv64-unknown-elf -march=rv64imac -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv64-unknown-elf -march=rv64imafc -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv64-unknown-elf -march=rv64imafdc -### %s -fsyntax-only 2>&1 | FileCheck %s
-
-// RUN: %clang -target riscv64-unknown-elf -march=rv64ia -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv64-unknown-elf -march=rv64iaf -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv64-unknown-elf -march=rv64iafd -### %s -fsyntax-only 2>&1 | FileCheck %s
-
-// RUN: %clang -target riscv64-unknown-elf -march=rv64iac -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv64-unknown-elf -march=rv64iafc -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv64-unknown-elf -march=rv64iafdc -### %s -fsyntax-only 2>&1 | FileCheck %s
-
-// RUN: %clang -target riscv64-unknown-elf -march=rv64g -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv64-unknown-elf -march=rv64gc -### %s -fsyntax-only 2>&1 | FileCheck %s
+// RUN: %clang -target riscv32-unknown-elf -march=rv32i -### %s \
+// RUN: -fsyntax-only 2>&1 | FileCheck %s
+// RUN: %clang -target riscv32-unknown-elf -march=rv32im -### %s \
+// RUN: -fsyntax-only 2>&1 | FileCheck %s
+// RUN: %clang -target riscv32-unknown-elf -march=rv32ima -### %s \
+// RUN: -fsyntax-only 2>&1 | FileCheck %s
+// RUN: %clang -target riscv32-unknown-elf -march=rv32imaf -### %s \
+// RUN: -fsyntax-only 2>&1 | FileCheck %s
+// RUN: %clang -target riscv32-unknown-elf -march=rv32imafd -### %s \
+// RUN: -fsyntax-only 2>&1 | FileCheck %s
+
+// RUN: %clang -target riscv32-unknown-elf -march=rv32ic -### %s \
+// RUN: -fsyntax-only 2>&1 | FileCheck %s
+// RUN: %clang -target riscv32-unknown-elf -march=rv32imc -### %s \
+// RUN: -fsyntax-only 2>&1 | FileCheck %s
+// RUN: %clang -target riscv32-unknown-elf -march=rv32imac -### %s \
+// RUN: -fsyntax-only 2>&1 | FileCheck %s
+// RUN: %clang 

[PATCH] D45284: [RISCV] More validations on the input value of -march=

[Ana Pazos via Phabricator via cfe-commits]

apazos added inline comments.



Comment at: lib/Driver/ToolChains/Arch/RISCV.cpp:50
+
+static void getExtensionVersion(StringRef In, std::string ) {
+  auto I = In.begin();

asb wrote:
> You should probably document the limitation that this doesn't currently parse 
> minor versions e.g. i2p0.
Correct, it is parsing the  major version for each standard extension. Will 
make note of how to parse minor version.



Comment at: lib/Driver/ToolChains/Arch/RISCV.cpp:144-146
+  // Currently LLVM does not support 'e'.
+  D.Diag(diag::err_drv_invalid_riscv_arch_name)
+<< MArch << "unsupported standard user-level extension 'e'";

asb wrote:
> This could be tightened up by also rejected rv64e as invalid.
OK, will add an error message for the invalid combo 'rv64' and 'e', though e is 
not supported yet for rv32.



Comment at: lib/Driver/ToolChains/Arch/RISCV.cpp:201-202
+  if (StdExtsItr == StdExtsEnd) {
+size_t Pos;
+if (hasExtension(StdExts, std::string(1, c), Pos)) {
+  D.Diag(diag::err_drv_invalid_riscv_ext_arch_name)

asb wrote:
> I'd suggest either just using StringRef::contains and getting rid of 
> hasExtension, or adding a doc comment to hasExtension to explain its 
> semantics.
> 
> It might also be worth adding a comment to explain why you want to check the 
> extension is present in the StdExts string (e.g. We have reached the end of 
> the StdExts string. Either the current extension was given outside of the 
> canonical order (in which case issue an error), or else no canonical ordering 
> is defined meaning no error should be generated'.
When we reach here, either c contains a valid extension but it was not given in
canonical order or it is an invalid extension. The code that follows was 
checking for the former, while the check for the latter happens in the switch 
default statement right below. But we can handle both here and print the 
appropriate messages, and leave the the check in switch statement just error 
out if LLVM does not support the extension. Yes, we can also get rid of 
hasExtension, it is not used any other place anymore.




Comment at: lib/Driver/ToolChains/Arch/RISCV.cpp:211
   // Move to next char to prevent repeated letter.
   ++StdExtsItr;
 

asb wrote:
> Won't this now iterate StdExtsItr past StdExtsEnd if StdExtsItr == StdExtsEnd 
> but the hasExtension call is false?
At this point, if StdExtsItr is StdExtsEnd, the code will error out in the 
switch case default statement. It means you found an invalid extension. 
Otherwise it will return due to invalid canonical order check above.

I willl move both error conditions to the same place to make the logic clearer.



Comment at: lib/Driver/ToolChains/Arch/RISCV.cpp:265-267
 if (HasD && !HasF)
-  D.Diag(diag::err_drv_invalid_arch_name) << MArch;
+  D.Diag(diag::err_drv_invalid_riscv_arch_name) << MArch
+<< "d requires f extension to also be specified";

asb wrote:
> Add a TODO about other dependencies perhaps? e.g. ef and efd are invalid, and 
> q requires rv64.
will make a note of the additional dependency checks.


https://reviews.llvm.org/D45284



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

[PATCH] D45284: [RISCV] More validations on the input value of -march=

[Ana Pazos via Phabricator via cfe-commits]

apazos updated this revision to Diff 142502.
apazos added a comment.

Fixed failure in release mode


https://reviews.llvm.org/D45284

Files:
  include/clang/Basic/DiagnosticDriverKinds.td
  lib/Driver/ToolChains/Arch/RISCV.cpp
  test/Driver/riscv-arch.c

Index: test/Driver/riscv-arch.c
===
--- test/Driver/riscv-arch.c
+++ test/Driver/riscv-arch.c
@@ -1,89 +1,245 @@
-// RUN: %clang -target riscv32-unknown-elf -march=rv32i -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv32-unknown-elf -march=rv32im -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv32-unknown-elf -march=rv32ima -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv32-unknown-elf -march=rv32imaf -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv32-unknown-elf -march=rv32imafd -### %s -fsyntax-only 2>&1 | FileCheck %s
-
-// RUN: %clang -target riscv32-unknown-elf -march=rv32ic -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv32-unknown-elf -march=rv32imc -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv32-unknown-elf -march=rv32imac -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv32-unknown-elf -march=rv32imafc -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv32-unknown-elf -march=rv32imafdc -### %s -fsyntax-only 2>&1 | FileCheck %s
-
-// RUN: %clang -target riscv32-unknown-elf -march=rv32ia -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv32-unknown-elf -march=rv32iaf -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv32-unknown-elf -march=rv32iafd -### %s -fsyntax-only 2>&1 | FileCheck %s
-
-// RUN: %clang -target riscv32-unknown-elf -march=rv32iac -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv32-unknown-elf -march=rv32iafc -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv32-unknown-elf -march=rv32iafdc -### %s -fsyntax-only 2>&1 | FileCheck %s
-
-// RUN: %clang -target riscv32-unknown-elf -march=rv32g -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv32-unknown-elf -march=rv32gc -### %s -fsyntax-only 2>&1 | FileCheck %s
-
-// RUN: %clang -target riscv64-unknown-elf -march=rv64i -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv64-unknown-elf -march=rv64im -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv64-unknown-elf -march=rv64ima -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv64-unknown-elf -march=rv64imaf -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv64-unknown-elf -march=rv64imafd -### %s -fsyntax-only 2>&1 | FileCheck %s
-
-// RUN: %clang -target riscv64-unknown-elf -march=rv64ic -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv64-unknown-elf -march=rv64imc -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv64-unknown-elf -march=rv64imac -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv64-unknown-elf -march=rv64imafc -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv64-unknown-elf -march=rv64imafdc -### %s -fsyntax-only 2>&1 | FileCheck %s
-
-// RUN: %clang -target riscv64-unknown-elf -march=rv64ia -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv64-unknown-elf -march=rv64iaf -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv64-unknown-elf -march=rv64iafd -### %s -fsyntax-only 2>&1 | FileCheck %s
-
-// RUN: %clang -target riscv64-unknown-elf -march=rv64iac -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv64-unknown-elf -march=rv64iafc -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv64-unknown-elf -march=rv64iafdc -### %s -fsyntax-only 2>&1 | FileCheck %s
-
-// RUN: %clang -target riscv64-unknown-elf -march=rv64g -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv64-unknown-elf -march=rv64gc -### %s -fsyntax-only 2>&1 | FileCheck %s
+// RUN: %clang -target riscv32-unknown-elf -march=rv32i -### %s \
+// RUN: -fsyntax-only 2>&1 | FileCheck %s
+// RUN: %clang -target riscv32-unknown-elf -march=rv32im -### %s \
+// RUN: -fsyntax-only 2>&1 | FileCheck %s
+// RUN: %clang -target riscv32-unknown-elf -march=rv32ima -### %s \
+// RUN: -fsyntax-only 2>&1 | FileCheck %s
+// RUN: %clang -target riscv32-unknown-elf -march=rv32imaf -### %s \
+// RUN: -fsyntax-only 2>&1 | FileCheck %s
+// RUN: %clang -target riscv32-unknown-elf -march=rv32imafd -### %s \
+// RUN: -fsyntax-only 2>&1 | FileCheck %s
+
+// RUN: %clang -target riscv32-unknown-elf -march=rv32ic -### %s \
+// RUN: -fsyntax-only 2>&1 | FileCheck %s
+// RUN: %clang -target riscv32-unknown-elf -march=rv32imc -### %s \
+// RUN: -fsyntax-only 2>&1 | FileCheck %s
+// RUN: %clang -target riscv32-unknown-elf -march=rv32imac -### %s \
+// RUN: -fsyntax-only 2>&1 | FileCheck %s
+// 

[PATCH] D45284: [RISCV] More validations on the input value of -march=

[Ana Pazos via Phabricator via cfe-commits]

apazos updated this revision to Diff 142459.
apazos added a comment.

- Simplified hasOtherExtensions() now that we clarified non-standard extensions 
are separated by '_'. We just need to find the first occurrence of the prefixes.
- updated error message, removed "unsupported" from some messages.


https://reviews.llvm.org/D45284

Files:
  include/clang/Basic/DiagnosticDriverKinds.td
  lib/Driver/ToolChains/Arch/RISCV.cpp
  test/Driver/riscv-arch.c

Index: test/Driver/riscv-arch.c
===
--- test/Driver/riscv-arch.c
+++ test/Driver/riscv-arch.c
@@ -1,89 +1,245 @@
-// RUN: %clang -target riscv32-unknown-elf -march=rv32i -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv32-unknown-elf -march=rv32im -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv32-unknown-elf -march=rv32ima -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv32-unknown-elf -march=rv32imaf -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv32-unknown-elf -march=rv32imafd -### %s -fsyntax-only 2>&1 | FileCheck %s
-
-// RUN: %clang -target riscv32-unknown-elf -march=rv32ic -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv32-unknown-elf -march=rv32imc -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv32-unknown-elf -march=rv32imac -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv32-unknown-elf -march=rv32imafc -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv32-unknown-elf -march=rv32imafdc -### %s -fsyntax-only 2>&1 | FileCheck %s
-
-// RUN: %clang -target riscv32-unknown-elf -march=rv32ia -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv32-unknown-elf -march=rv32iaf -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv32-unknown-elf -march=rv32iafd -### %s -fsyntax-only 2>&1 | FileCheck %s
-
-// RUN: %clang -target riscv32-unknown-elf -march=rv32iac -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv32-unknown-elf -march=rv32iafc -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv32-unknown-elf -march=rv32iafdc -### %s -fsyntax-only 2>&1 | FileCheck %s
-
-// RUN: %clang -target riscv32-unknown-elf -march=rv32g -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv32-unknown-elf -march=rv32gc -### %s -fsyntax-only 2>&1 | FileCheck %s
-
-// RUN: %clang -target riscv64-unknown-elf -march=rv64i -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv64-unknown-elf -march=rv64im -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv64-unknown-elf -march=rv64ima -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv64-unknown-elf -march=rv64imaf -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv64-unknown-elf -march=rv64imafd -### %s -fsyntax-only 2>&1 | FileCheck %s
-
-// RUN: %clang -target riscv64-unknown-elf -march=rv64ic -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv64-unknown-elf -march=rv64imc -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv64-unknown-elf -march=rv64imac -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv64-unknown-elf -march=rv64imafc -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv64-unknown-elf -march=rv64imafdc -### %s -fsyntax-only 2>&1 | FileCheck %s
-
-// RUN: %clang -target riscv64-unknown-elf -march=rv64ia -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv64-unknown-elf -march=rv64iaf -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv64-unknown-elf -march=rv64iafd -### %s -fsyntax-only 2>&1 | FileCheck %s
-
-// RUN: %clang -target riscv64-unknown-elf -march=rv64iac -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv64-unknown-elf -march=rv64iafc -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv64-unknown-elf -march=rv64iafdc -### %s -fsyntax-only 2>&1 | FileCheck %s
-
-// RUN: %clang -target riscv64-unknown-elf -march=rv64g -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv64-unknown-elf -march=rv64gc -### %s -fsyntax-only 2>&1 | FileCheck %s
+// RUN: %clang -target riscv32-unknown-elf -march=rv32i -### %s \
+// RUN: -fsyntax-only 2>&1 | FileCheck %s
+// RUN: %clang -target riscv32-unknown-elf -march=rv32im -### %s \
+// RUN: -fsyntax-only 2>&1 | FileCheck %s
+// RUN: %clang -target riscv32-unknown-elf -march=rv32ima -### %s \
+// RUN: -fsyntax-only 2>&1 | FileCheck %s
+// RUN: %clang -target riscv32-unknown-elf -march=rv32imaf -### %s \
+// RUN: -fsyntax-only 2>&1 | FileCheck %s
+// RUN: %clang -target riscv32-unknown-elf -march=rv32imafd -### %s \
+// RUN: -fsyntax-only 2>&1 | FileCheck %s
+
+// RUN: %clang -target riscv32-unknown-elf -march=rv32ic -### %s \
+// RUN: -fsyntax-only 2>&1 | FileCheck %s
+// RUN: %clang -target 

[PATCH] D45284: [RISCV] More validations on the input value of -march=

[Ana Pazos via Phabricator via cfe-commits]

apazos updated this revision to Diff 142279.
apazos added a comment.

Updated error messages and fixed getExtensionVersion


https://reviews.llvm.org/D45284

Files:
  include/clang/Basic/DiagnosticDriverKinds.td
  lib/Driver/ToolChains/Arch/RISCV.cpp
  test/Driver/riscv-arch.c

Index: test/Driver/riscv-arch.c
===
--- test/Driver/riscv-arch.c
+++ test/Driver/riscv-arch.c
@@ -1,89 +1,245 @@
-// RUN: %clang -target riscv32-unknown-elf -march=rv32i -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv32-unknown-elf -march=rv32im -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv32-unknown-elf -march=rv32ima -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv32-unknown-elf -march=rv32imaf -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv32-unknown-elf -march=rv32imafd -### %s -fsyntax-only 2>&1 | FileCheck %s
-
-// RUN: %clang -target riscv32-unknown-elf -march=rv32ic -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv32-unknown-elf -march=rv32imc -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv32-unknown-elf -march=rv32imac -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv32-unknown-elf -march=rv32imafc -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv32-unknown-elf -march=rv32imafdc -### %s -fsyntax-only 2>&1 | FileCheck %s
-
-// RUN: %clang -target riscv32-unknown-elf -march=rv32ia -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv32-unknown-elf -march=rv32iaf -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv32-unknown-elf -march=rv32iafd -### %s -fsyntax-only 2>&1 | FileCheck %s
-
-// RUN: %clang -target riscv32-unknown-elf -march=rv32iac -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv32-unknown-elf -march=rv32iafc -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv32-unknown-elf -march=rv32iafdc -### %s -fsyntax-only 2>&1 | FileCheck %s
-
-// RUN: %clang -target riscv32-unknown-elf -march=rv32g -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv32-unknown-elf -march=rv32gc -### %s -fsyntax-only 2>&1 | FileCheck %s
-
-// RUN: %clang -target riscv64-unknown-elf -march=rv64i -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv64-unknown-elf -march=rv64im -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv64-unknown-elf -march=rv64ima -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv64-unknown-elf -march=rv64imaf -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv64-unknown-elf -march=rv64imafd -### %s -fsyntax-only 2>&1 | FileCheck %s
-
-// RUN: %clang -target riscv64-unknown-elf -march=rv64ic -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv64-unknown-elf -march=rv64imc -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv64-unknown-elf -march=rv64imac -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv64-unknown-elf -march=rv64imafc -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv64-unknown-elf -march=rv64imafdc -### %s -fsyntax-only 2>&1 | FileCheck %s
-
-// RUN: %clang -target riscv64-unknown-elf -march=rv64ia -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv64-unknown-elf -march=rv64iaf -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv64-unknown-elf -march=rv64iafd -### %s -fsyntax-only 2>&1 | FileCheck %s
-
-// RUN: %clang -target riscv64-unknown-elf -march=rv64iac -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv64-unknown-elf -march=rv64iafc -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv64-unknown-elf -march=rv64iafdc -### %s -fsyntax-only 2>&1 | FileCheck %s
-
-// RUN: %clang -target riscv64-unknown-elf -march=rv64g -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv64-unknown-elf -march=rv64gc -### %s -fsyntax-only 2>&1 | FileCheck %s
+// RUN: %clang -target riscv32-unknown-elf -march=rv32i -### %s \
+// RUN: -fsyntax-only 2>&1 | FileCheck %s
+// RUN: %clang -target riscv32-unknown-elf -march=rv32im -### %s \
+// RUN: -fsyntax-only 2>&1 | FileCheck %s
+// RUN: %clang -target riscv32-unknown-elf -march=rv32ima -### %s \
+// RUN: -fsyntax-only 2>&1 | FileCheck %s
+// RUN: %clang -target riscv32-unknown-elf -march=rv32imaf -### %s \
+// RUN: -fsyntax-only 2>&1 | FileCheck %s
+// RUN: %clang -target riscv32-unknown-elf -march=rv32imafd -### %s \
+// RUN: -fsyntax-only 2>&1 | FileCheck %s
+
+// RUN: %clang -target riscv32-unknown-elf -march=rv32ic -### %s \
+// RUN: -fsyntax-only 2>&1 | FileCheck %s
+// RUN: %clang -target riscv32-unknown-elf -march=rv32imc -### %s \
+// RUN: -fsyntax-only 2>&1 | FileCheck %s
+// RUN: %clang -target riscv32-unknown-elf -march=rv32imac -### %s \
+// RUN: -fsyntax-only 

[PATCH] D45284: [RISCV] More validations on the input value of -march=

[Ana Pazos via Phabricator via cfe-commits]

apazos updated this revision to Diff 141717.
apazos edited the summary of this revision.
apazos added a comment.
Herald added a subscriber: zzheng.

Updated code according to the ISA string rules that have been clarified.


https://reviews.llvm.org/D45284

Files:
  include/clang/Basic/DiagnosticDriverKinds.td
  lib/Driver/ToolChains/Arch/RISCV.cpp
  test/Driver/riscv-arch.c

Index: test/Driver/riscv-arch.c
===
--- test/Driver/riscv-arch.c
+++ test/Driver/riscv-arch.c
@@ -1,89 +1,243 @@
-// RUN: %clang -target riscv32-unknown-elf -march=rv32i -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv32-unknown-elf -march=rv32im -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv32-unknown-elf -march=rv32ima -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv32-unknown-elf -march=rv32imaf -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv32-unknown-elf -march=rv32imafd -### %s -fsyntax-only 2>&1 | FileCheck %s
-
-// RUN: %clang -target riscv32-unknown-elf -march=rv32ic -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv32-unknown-elf -march=rv32imc -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv32-unknown-elf -march=rv32imac -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv32-unknown-elf -march=rv32imafc -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv32-unknown-elf -march=rv32imafdc -### %s -fsyntax-only 2>&1 | FileCheck %s
-
-// RUN: %clang -target riscv32-unknown-elf -march=rv32ia -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv32-unknown-elf -march=rv32iaf -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv32-unknown-elf -march=rv32iafd -### %s -fsyntax-only 2>&1 | FileCheck %s
-
-// RUN: %clang -target riscv32-unknown-elf -march=rv32iac -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv32-unknown-elf -march=rv32iafc -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv32-unknown-elf -march=rv32iafdc -### %s -fsyntax-only 2>&1 | FileCheck %s
-
-// RUN: %clang -target riscv32-unknown-elf -march=rv32g -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv32-unknown-elf -march=rv32gc -### %s -fsyntax-only 2>&1 | FileCheck %s
-
-// RUN: %clang -target riscv64-unknown-elf -march=rv64i -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv64-unknown-elf -march=rv64im -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv64-unknown-elf -march=rv64ima -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv64-unknown-elf -march=rv64imaf -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv64-unknown-elf -march=rv64imafd -### %s -fsyntax-only 2>&1 | FileCheck %s
-
-// RUN: %clang -target riscv64-unknown-elf -march=rv64ic -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv64-unknown-elf -march=rv64imc -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv64-unknown-elf -march=rv64imac -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv64-unknown-elf -march=rv64imafc -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv64-unknown-elf -march=rv64imafdc -### %s -fsyntax-only 2>&1 | FileCheck %s
-
-// RUN: %clang -target riscv64-unknown-elf -march=rv64ia -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv64-unknown-elf -march=rv64iaf -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv64-unknown-elf -march=rv64iafd -### %s -fsyntax-only 2>&1 | FileCheck %s
-
-// RUN: %clang -target riscv64-unknown-elf -march=rv64iac -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv64-unknown-elf -march=rv64iafc -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv64-unknown-elf -march=rv64iafdc -### %s -fsyntax-only 2>&1 | FileCheck %s
-
-// RUN: %clang -target riscv64-unknown-elf -march=rv64g -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv64-unknown-elf -march=rv64gc -### %s -fsyntax-only 2>&1 | FileCheck %s
+// RUN: %clang -target riscv32-unknown-elf -march=rv32i -### %s \
+// RUN: -fsyntax-only 2>&1 | FileCheck %s
+// RUN: %clang -target riscv32-unknown-elf -march=rv32im -### %s \
+// RUN: -fsyntax-only 2>&1 | FileCheck %s
+// RUN: %clang -target riscv32-unknown-elf -march=rv32ima -### %s \
+// RUN: -fsyntax-only 2>&1 | FileCheck %s
+// RUN: %clang -target riscv32-unknown-elf -march=rv32imaf -### %s \
+// RUN: -fsyntax-only 2>&1 | FileCheck %s
+// RUN: %clang -target riscv32-unknown-elf -march=rv32imafd -### %s \
+// RUN: -fsyntax-only 2>&1 | FileCheck %s
+
+// RUN: %clang -target riscv32-unknown-elf -march=rv32ic -### %s \
+// RUN: -fsyntax-only 2>&1 | FileCheck %s
+// RUN: %clang -target riscv32-unknown-elf -march=rv32imc -### %s \
+// RUN: -fsyntax-only 2>&1 | 

[PATCH] D45284: [RISCV] More validations on the input value of -march=

[Ana Pazos via Phabricator via cfe-commits]

apazos updated this revision to Diff 141050.
apazos added a comment.

fixed test merged line


https://reviews.llvm.org/D45284

Files:
  include/clang/Basic/DiagnosticDriverKinds.td
  lib/Driver/ToolChains/Arch/RISCV.cpp
  test/Driver/riscv-arch.c

Index: test/Driver/riscv-arch.c
===
--- test/Driver/riscv-arch.c
+++ test/Driver/riscv-arch.c
@@ -1,89 +1,229 @@
-// RUN: %clang -target riscv32-unknown-elf -march=rv32i -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv32-unknown-elf -march=rv32im -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv32-unknown-elf -march=rv32ima -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv32-unknown-elf -march=rv32imaf -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv32-unknown-elf -march=rv32imafd -### %s -fsyntax-only 2>&1 | FileCheck %s
-
-// RUN: %clang -target riscv32-unknown-elf -march=rv32ic -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv32-unknown-elf -march=rv32imc -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv32-unknown-elf -march=rv32imac -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv32-unknown-elf -march=rv32imafc -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv32-unknown-elf -march=rv32imafdc -### %s -fsyntax-only 2>&1 | FileCheck %s
-
-// RUN: %clang -target riscv32-unknown-elf -march=rv32ia -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv32-unknown-elf -march=rv32iaf -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv32-unknown-elf -march=rv32iafd -### %s -fsyntax-only 2>&1 | FileCheck %s
-
-// RUN: %clang -target riscv32-unknown-elf -march=rv32iac -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv32-unknown-elf -march=rv32iafc -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv32-unknown-elf -march=rv32iafdc -### %s -fsyntax-only 2>&1 | FileCheck %s
-
-// RUN: %clang -target riscv32-unknown-elf -march=rv32g -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv32-unknown-elf -march=rv32gc -### %s -fsyntax-only 2>&1 | FileCheck %s
-
-// RUN: %clang -target riscv64-unknown-elf -march=rv64i -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv64-unknown-elf -march=rv64im -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv64-unknown-elf -march=rv64ima -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv64-unknown-elf -march=rv64imaf -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv64-unknown-elf -march=rv64imafd -### %s -fsyntax-only 2>&1 | FileCheck %s
-
-// RUN: %clang -target riscv64-unknown-elf -march=rv64ic -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv64-unknown-elf -march=rv64imc -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv64-unknown-elf -march=rv64imac -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv64-unknown-elf -march=rv64imafc -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv64-unknown-elf -march=rv64imafdc -### %s -fsyntax-only 2>&1 | FileCheck %s
-
-// RUN: %clang -target riscv64-unknown-elf -march=rv64ia -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv64-unknown-elf -march=rv64iaf -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv64-unknown-elf -march=rv64iafd -### %s -fsyntax-only 2>&1 | FileCheck %s
-
-// RUN: %clang -target riscv64-unknown-elf -march=rv64iac -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv64-unknown-elf -march=rv64iafc -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv64-unknown-elf -march=rv64iafdc -### %s -fsyntax-only 2>&1 | FileCheck %s
-
-// RUN: %clang -target riscv64-unknown-elf -march=rv64g -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv64-unknown-elf -march=rv64gc -### %s -fsyntax-only 2>&1 | FileCheck %s
+// RUN: %clang -target riscv32-unknown-elf -march=rv32i -### %s \
+// RUN: -fsyntax-only 2>&1 | FileCheck %s
+// RUN: %clang -target riscv32-unknown-elf -march=rv32im -### %s \
+// RUN: -fsyntax-only 2>&1 | FileCheck %s
+// RUN: %clang -target riscv32-unknown-elf -march=rv32ima -### %s \
+// RUN: -fsyntax-only 2>&1 | FileCheck %s
+// RUN: %clang -target riscv32-unknown-elf -march=rv32imaf -### %s \
+// RUN: -fsyntax-only 2>&1 | FileCheck %s
+// RUN: %clang -target riscv32-unknown-elf -march=rv32imafd -### %s \
+// RUN: -fsyntax-only 2>&1 | FileCheck %s
+
+// RUN: %clang -target riscv32-unknown-elf -march=rv32ic -### %s \
+// RUN: -fsyntax-only 2>&1 | FileCheck %s
+// RUN: %clang -target riscv32-unknown-elf -march=rv32imc -### %s \
+// RUN: -fsyntax-only 2>&1 | FileCheck %s
+// RUN: %clang -target riscv32-unknown-elf -march=rv32imac -### %s \
+// RUN: -fsyntax-only 2>&1 | FileCheck %s
+// RUN: 

[PATCH] D45284: [RISCV] More validations on the input value of -march=

[Ana Pazos via Phabricator via cfe-commits]

apazos updated this revision to Diff 141048.
apazos added a comment.

test fix


https://reviews.llvm.org/D45284

Files:
  include/clang/Basic/DiagnosticDriverKinds.td
  lib/Driver/ToolChains/Arch/RISCV.cpp
  test/Driver/riscv-arch.c

Index: test/Driver/riscv-arch.c
===
--- test/Driver/riscv-arch.c
+++ test/Driver/riscv-arch.c
@@ -1,89 +1,229 @@
-// RUN: %clang -target riscv32-unknown-elf -march=rv32i -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv32-unknown-elf -march=rv32im -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv32-unknown-elf -march=rv32ima -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv32-unknown-elf -march=rv32imaf -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv32-unknown-elf -march=rv32imafd -### %s -fsyntax-only 2>&1 | FileCheck %s
-
-// RUN: %clang -target riscv32-unknown-elf -march=rv32ic -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv32-unknown-elf -march=rv32imc -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv32-unknown-elf -march=rv32imac -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv32-unknown-elf -march=rv32imafc -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv32-unknown-elf -march=rv32imafdc -### %s -fsyntax-only 2>&1 | FileCheck %s
-
-// RUN: %clang -target riscv32-unknown-elf -march=rv32ia -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv32-unknown-elf -march=rv32iaf -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv32-unknown-elf -march=rv32iafd -### %s -fsyntax-only 2>&1 | FileCheck %s
-
-// RUN: %clang -target riscv32-unknown-elf -march=rv32iac -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv32-unknown-elf -march=rv32iafc -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv32-unknown-elf -march=rv32iafdc -### %s -fsyntax-only 2>&1 | FileCheck %s
-
-// RUN: %clang -target riscv32-unknown-elf -march=rv32g -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv32-unknown-elf -march=rv32gc -### %s -fsyntax-only 2>&1 | FileCheck %s
-
-// RUN: %clang -target riscv64-unknown-elf -march=rv64i -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv64-unknown-elf -march=rv64im -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv64-unknown-elf -march=rv64ima -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv64-unknown-elf -march=rv64imaf -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv64-unknown-elf -march=rv64imafd -### %s -fsyntax-only 2>&1 | FileCheck %s
-
-// RUN: %clang -target riscv64-unknown-elf -march=rv64ic -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv64-unknown-elf -march=rv64imc -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv64-unknown-elf -march=rv64imac -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv64-unknown-elf -march=rv64imafc -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv64-unknown-elf -march=rv64imafdc -### %s -fsyntax-only 2>&1 | FileCheck %s
-
-// RUN: %clang -target riscv64-unknown-elf -march=rv64ia -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv64-unknown-elf -march=rv64iaf -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv64-unknown-elf -march=rv64iafd -### %s -fsyntax-only 2>&1 | FileCheck %s
-
-// RUN: %clang -target riscv64-unknown-elf -march=rv64iac -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv64-unknown-elf -march=rv64iafc -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv64-unknown-elf -march=rv64iafdc -### %s -fsyntax-only 2>&1 | FileCheck %s
-
-// RUN: %clang -target riscv64-unknown-elf -march=rv64g -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv64-unknown-elf -march=rv64gc -### %s -fsyntax-only 2>&1 | FileCheck %s
+// RUN: %clang -target riscv32-unknown-elf -march=rv32i -### %s \
+// RUN: -fsyntax-only 2>&1 | FileCheck %s
+// RUN: %clang -target riscv32-unknown-elf -march=rv32im -### %s \
+// RUN: -fsyntax-only 2>&1 | FileCheck %s
+// RUN: %clang -target riscv32-unknown-elf -march=rv32ima -### %s \
+// RUN: -fsyntax-only 2>&1 | FileCheck %s
+// RUN: %clang -target riscv32-unknown-elf -march=rv32imaf -### %s \
+// RUN: -fsyntax-only 2>&1 | FileCheck %s
+// RUN: %clang -target riscv32-unknown-elf -march=rv32imafd -### %s \
+// RUN: -fsyntax-only 2>&1 | FileCheck %s
+
+// RUN: %clang -target riscv32-unknown-elf -march=rv32ic -### %s \
+// RUN: -fsyntax-only 2>&1 | FileCheck %s
+// RUN: %clang -target riscv32-unknown-elf -march=rv32imc -### %s \
+// RUN: -fsyntax-only RUN: 2>&1 | FileCheck %s
+// RUN: %clang -target riscv32-unknown-elf -march=rv32imac -### %s \
+// RUN: -fsyntax-only 2>&1 | FileCheck %s
+// RUN: %clang 

[PATCH] D45284: [RISCV] More validations on the input value of -march=

[Ana Pazos via Phabricator via cfe-commits]

apazos updated this revision to Diff 141043.
apazos added a comment.

updated test case


https://reviews.llvm.org/D45284

Files:
  include/clang/Basic/DiagnosticDriverKinds.td
  lib/Driver/ToolChains/Arch/RISCV.cpp
  test/Driver/riscv-arch.c

Index: test/Driver/riscv-arch.c
===
--- test/Driver/riscv-arch.c
+++ test/Driver/riscv-arch.c
@@ -1,89 +1,229 @@
-// RUN: %clang -target riscv32-unknown-elf -march=rv32i -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv32-unknown-elf -march=rv32im -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv32-unknown-elf -march=rv32ima -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv32-unknown-elf -march=rv32imaf -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv32-unknown-elf -march=rv32imafd -### %s -fsyntax-only 2>&1 | FileCheck %s
-
-// RUN: %clang -target riscv32-unknown-elf -march=rv32ic -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv32-unknown-elf -march=rv32imc -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv32-unknown-elf -march=rv32imac -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv32-unknown-elf -march=rv32imafc -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv32-unknown-elf -march=rv32imafdc -### %s -fsyntax-only 2>&1 | FileCheck %s
-
-// RUN: %clang -target riscv32-unknown-elf -march=rv32ia -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv32-unknown-elf -march=rv32iaf -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv32-unknown-elf -march=rv32iafd -### %s -fsyntax-only 2>&1 | FileCheck %s
-
-// RUN: %clang -target riscv32-unknown-elf -march=rv32iac -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv32-unknown-elf -march=rv32iafc -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv32-unknown-elf -march=rv32iafdc -### %s -fsyntax-only 2>&1 | FileCheck %s
-
-// RUN: %clang -target riscv32-unknown-elf -march=rv32g -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv32-unknown-elf -march=rv32gc -### %s -fsyntax-only 2>&1 | FileCheck %s
-
-// RUN: %clang -target riscv64-unknown-elf -march=rv64i -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv64-unknown-elf -march=rv64im -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv64-unknown-elf -march=rv64ima -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv64-unknown-elf -march=rv64imaf -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv64-unknown-elf -march=rv64imafd -### %s -fsyntax-only 2>&1 | FileCheck %s
-
-// RUN: %clang -target riscv64-unknown-elf -march=rv64ic -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv64-unknown-elf -march=rv64imc -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv64-unknown-elf -march=rv64imac -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv64-unknown-elf -march=rv64imafc -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv64-unknown-elf -march=rv64imafdc -### %s -fsyntax-only 2>&1 | FileCheck %s
-
-// RUN: %clang -target riscv64-unknown-elf -march=rv64ia -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv64-unknown-elf -march=rv64iaf -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv64-unknown-elf -march=rv64iafd -### %s -fsyntax-only 2>&1 | FileCheck %s
-
-// RUN: %clang -target riscv64-unknown-elf -march=rv64iac -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv64-unknown-elf -march=rv64iafc -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv64-unknown-elf -march=rv64iafdc -### %s -fsyntax-only 2>&1 | FileCheck %s
-
-// RUN: %clang -target riscv64-unknown-elf -march=rv64g -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv64-unknown-elf -march=rv64gc -### %s -fsyntax-only 2>&1 | FileCheck %s
+// RUN: %clang -target riscv32-unknown-elf -march=rv32i -### %s \
+// RUN: -fsyntax-only 2>&1 | FileCheck %s
+// RUN: %clang -target riscv32-unknown-elf -march=rv32im -### %s \
+// RUN: -fsyntax-only 2>&1 | FileCheck %s
+// RUN: %clang -target riscv32-unknown-elf -march=rv32ima -### %s \
+// RUN -fsyntax-only 2>&1 | FileCheck %s
+// RUN: %clang -target riscv32-unknown-elf -march=rv32imaf -### %s \
+// RUN: -fsyntax-only 2>&1 | FileCheck %s
+// RUN: %clang -target riscv32-unknown-elf -march=rv32imafd -### %s \
+// RUN: -fsyntax-only 2>&1 | FileCheck %s
+
+// RUN: %clang -target riscv32-unknown-elf -march=rv32ic -### %s \
+// RUN: -fsyntax-only 2>&1 | FileCheck %s
+// RUN: %clang -target riscv32-unknown-elf -march=rv32imc -### %s \
+// RUN: -fsyntax-only RUN: 2>&1 | FileCheck %s
+// RUN: %clang -target riscv32-unknown-elf -march=rv32imac -### %s \
+// RUN: -fsyntax-only 2>&1 | FileCheck %s
+// RUN: 

[PATCH] D45284: [RISCV] More validations on the input value of -march=

[Ana Pazos via Phabricator via cfe-commits]

apazos created this revision.
apazos added reviewers: asb, kito-cheng.
Herald added subscribers: shiva0217, niosHD, sabuasal, jordy.potman.lists, 
simoncook, johnrusso, rbar.

- Updated diagnostic messages for invalid/unsupported  march combinations.
- Parsing X, SX and S extensions.


https://reviews.llvm.org/D45284

Files:
  include/clang/Basic/DiagnosticDriverKinds.td
  lib/Driver/ToolChains/Arch/RISCV.cpp
  test/Driver/riscv-arch.c

Index: test/Driver/riscv-arch.c
===
--- test/Driver/riscv-arch.c
+++ test/Driver/riscv-arch.c
@@ -1,89 +1,229 @@
-// RUN: %clang -target riscv32-unknown-elf -march=rv32i -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv32-unknown-elf -march=rv32im -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv32-unknown-elf -march=rv32ima -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv32-unknown-elf -march=rv32imaf -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv32-unknown-elf -march=rv32imafd -### %s -fsyntax-only 2>&1 | FileCheck %s
-
-// RUN: %clang -target riscv32-unknown-elf -march=rv32ic -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv32-unknown-elf -march=rv32imc -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv32-unknown-elf -march=rv32imac -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv32-unknown-elf -march=rv32imafc -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv32-unknown-elf -march=rv32imafdc -### %s -fsyntax-only 2>&1 | FileCheck %s
-
-// RUN: %clang -target riscv32-unknown-elf -march=rv32ia -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv32-unknown-elf -march=rv32iaf -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv32-unknown-elf -march=rv32iafd -### %s -fsyntax-only 2>&1 | FileCheck %s
-
-// RUN: %clang -target riscv32-unknown-elf -march=rv32iac -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv32-unknown-elf -march=rv32iafc -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv32-unknown-elf -march=rv32iafdc -### %s -fsyntax-only 2>&1 | FileCheck %s
-
-// RUN: %clang -target riscv32-unknown-elf -march=rv32g -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv32-unknown-elf -march=rv32gc -### %s -fsyntax-only 2>&1 | FileCheck %s
-
-// RUN: %clang -target riscv64-unknown-elf -march=rv64i -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv64-unknown-elf -march=rv64im -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv64-unknown-elf -march=rv64ima -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv64-unknown-elf -march=rv64imaf -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv64-unknown-elf -march=rv64imafd -### %s -fsyntax-only 2>&1 | FileCheck %s
-
-// RUN: %clang -target riscv64-unknown-elf -march=rv64ic -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv64-unknown-elf -march=rv64imc -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv64-unknown-elf -march=rv64imac -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv64-unknown-elf -march=rv64imafc -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv64-unknown-elf -march=rv64imafdc -### %s -fsyntax-only 2>&1 | FileCheck %s
-
-// RUN: %clang -target riscv64-unknown-elf -march=rv64ia -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv64-unknown-elf -march=rv64iaf -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv64-unknown-elf -march=rv64iafd -### %s -fsyntax-only 2>&1 | FileCheck %s
-
-// RUN: %clang -target riscv64-unknown-elf -march=rv64iac -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv64-unknown-elf -march=rv64iafc -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv64-unknown-elf -march=rv64iafdc -### %s -fsyntax-only 2>&1 | FileCheck %s
-
-// RUN: %clang -target riscv64-unknown-elf -march=rv64g -### %s -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -target riscv64-unknown-elf -march=rv64gc -### %s -fsyntax-only 2>&1 | FileCheck %s
+// RUN: %clang -target riscv32-unknown-elf -march=rv32i -### %s \
+// RUN: -fsyntax-only 2>&1 | FileCheck %s
+// RUN: %clang -target riscv32-unknown-elf -march=rv32im -### %s \
+RUN: -fsyntax-only 2>&1 | FileCheck %s
+// RUN: %clang -target riscv32-unknown-elf -march=rv32ima -### %s \
+// RUN -fsyntax-only 2>&1 | FileCheck %s
+// RUN: %clang -target riscv32-unknown-elf -march=rv32imaf -### %s \
+// RUN: -fsyntax-only 2>&1 | FileCheck %s
+// RUN: %clang -target riscv32-unknown-elf -march=rv32imafd -### %s \
+// RUN: -fsyntax-only 2>&1 | FileCheck %s
+
+// RUN: %clang -target riscv32-unknown-elf -march=rv32ic -### %s \
+// RUN: -fsyntax-only 2>&1 | FileCheck %s
+// RUN: %clang -target riscv32-unknown-elf 

[PATCH] D44888: [RISCV] Default enable linker relaxation and add -mno-relax flag to disable it

[Ana Pazos via Phabricator via cfe-commits]

apazos added inline comments.



Comment at: include/clang/Driver/Options.td:1874
 
+def mno_relax : Flag<["-"], "mno-relax">,
+Group;

I think we should define both -mrelax and -mno-relax flags



Comment at: lib/Driver/ToolChains/Arch/RISCV.cpp:55
   }
+  if (Args.getLastArg(options::OPT_mno_relax)) {
+// Not passing relax.

If we add both mrelax and mno-relax flags, we need to update this logic...

// -mrelax is default, unless -mno-relax is specified.
bool Relax = true;
if (auto *A = Args.getLastArg(options::OPT_mrelax, options::OPT_mno_relax))
  if(A->getOption().matches(options::OPT_mno_relax))
Relax = false;
if (Relax)
 Features.push_back("+relax");




Repository:
  rC Clang

https://reviews.llvm.org/D44888



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

[PATCH] D44727: [RISCV] Implement getTargetDefines, handleTargetFeatures and hasFeature for RISCVTargetInfo

[Ana Pazos via Phabricator via cfe-commits]

apazos added inline comments.



Comment at: test/Preprocessor/riscv-target-features.c:9
+
+// RUN: %clang -target riscv32-unknown-linux-gnu -march=rv32im -x c -E -dM %s 
-o - | FileCheck --check-prefix=CHECK-M-EXT %s
+// RUN: %clang -target riscv64-unknown-linux-gnu -march=rv64im -x c -E -dM %s 
-o - | FileCheck --check-prefix=CHECK-M-EXT %s

Consider breaking  the long lines to be within 80 cols.


Repository:
  rC Clang

https://reviews.llvm.org/D44727



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

[PATCH] D44189: [RISCV] Verify the input value of -march=

[Ana Pazos via Phabricator via cfe-commits]

apazos added inline comments.



Comment at: lib/Driver/ToolChains/Arch/RISCV.cpp:51
+  break;
+case 'g':
+  // g = imafd

One more question - how about non-standard extensions (vendor/custom) prefixed 
with X?
Shouldn't we add the logic to process 'Xext' occurrences in the march string as 
part of this patch?



Repository:
  rC Clang

https://reviews.llvm.org/D44189



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

[PATCH] D44189: [RISCV] Verify the input value of -march=

[Ana Pazos via Phabricator via cfe-commits]

apazos added inline comments.



Comment at: lib/Driver/ToolChains/Arch/RISCV.cpp:48
+  break;
+default:
+  // First letter should be 'i' or 'g'.

In the switch cases move default to first position.



Comment at: lib/Driver/ToolChains/Arch/RISCV.cpp:60
   case 'm':
 Features.push_back("+m");
 break;

So the subsequent features can appear in any order?


Repository:
  rC Clang

https://reviews.llvm.org/D44189



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

[PATCH] D43105: [RISCV] Enable __int128_t and uint128_t through clang flag

[Ana Pazos via Phabricator via cfe-commits]

apazos added inline comments.



Comment at: lib/Basic/Targets/RISCV.h:85
+  bool hasInt128Type(const LangOptions ) const override {
+return Opts.UseInt128;
+  }

kito-cheng wrote:
> efriedma wrote:
> > Maybe make this a cross-platform flag, rather than riscv-specific?
> +1, then we can make all other 32 bits target to able easier support float128 
> too :)
OK... so we can move the option check to the TargetInfo class, and the 
target-specific implementation overrides it as needed.


Repository:
  rC Clang

https://reviews.llvm.org/D43105



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

[PATCH] D42666: Revert "Revert rC322769: [RISCV] Propagate -mabi and -march values to GNU assembler."

[Ana Pazos via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC323894: Revert Revert rC322769: [RISCV] Propagate 
-mabi and -march values to GNU… (authored by apazos, committed by ).

Repository:
  rL LLVM

https://reviews.llvm.org/D42666

Files:
  lib/Driver/ToolChains/Gnu.cpp
  test/Driver/riscv-gnutools.c


Index: test/Driver/riscv-gnutools.c
===
--- test/Driver/riscv-gnutools.c
+++ test/Driver/riscv-gnutools.c
@@ -0,0 +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: %clang -target riscv32 -fno-integrated-as -march=rv32g %s -### -c \
+// RUN: 2>&1 | FileCheck -check-prefix=MABI-ILP32-MARCH-G %s
+
+// RUN: %clang -target riscv64 -fno-integrated-as %s -###  -c \
+// RUN: 2>&1 | FileCheck -check-prefix=MABI-ILP64 %s
+
+// RUN: %clang -target riscv64 -fno-integrated-as -march=rv64g %s -### -c \
+// RUN: 2>&1 | FileCheck -check-prefix=MABI-ILP64-MARCH-G %s
+
+// MABI-ILP32: "{{.*}}as{{(.exe)?}}" "-mabi" "ilp32"
+// MABI-ILP32-MARCH-G: "{{.*}}as{{(.exe)?}}" "-mabi" "ilp32" "-march" "rv32g"
+
+// MABI-ILP64: "{{.*}}as{{(.exe)?}}" "-mabi" "lp64"
+// MABI-ILP64-MARCH-G: "{{.*}}as{{(.exe)?}}" "-mabi" "lp64" "-march" "rv64g"
Index: lib/Driver/ToolChains/Gnu.cpp
===
--- lib/Driver/ToolChains/Gnu.cpp
+++ lib/Driver/ToolChains/Gnu.cpp
@@ -629,6 +629,18 @@
   ppc::getPPCAsmModeForCPU(getCPUName(Args, getToolChain().getTriple(;
 break;
   }
+  case llvm::Triple::riscv32:
+  case llvm::Triple::riscv64: {
+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());
+}
+break;
+  }
   case llvm::Triple::sparc:
   case llvm::Triple::sparcel: {
 CmdArgs.push_back("-32");


Index: test/Driver/riscv-gnutools.c
===
--- test/Driver/riscv-gnutools.c
+++ test/Driver/riscv-gnutools.c
@@ -0,0 +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: %clang -target riscv32 -fno-integrated-as -march=rv32g %s -### -c \
+// RUN: 2>&1 | FileCheck -check-prefix=MABI-ILP32-MARCH-G %s
+
+// RUN: %clang -target riscv64 -fno-integrated-as %s -###  -c \
+// RUN: 2>&1 | FileCheck -check-prefix=MABI-ILP64 %s
+
+// RUN: %clang -target riscv64 -fno-integrated-as -march=rv64g %s -### -c \
+// RUN: 2>&1 | FileCheck -check-prefix=MABI-ILP64-MARCH-G %s
+
+// MABI-ILP32: "{{.*}}as{{(.exe)?}}" "-mabi" "ilp32"
+// MABI-ILP32-MARCH-G: "{{.*}}as{{(.exe)?}}" "-mabi" "ilp32" "-march" "rv32g"
+
+// MABI-ILP64: "{{.*}}as{{(.exe)?}}" "-mabi" "lp64"
+// MABI-ILP64-MARCH-G: "{{.*}}as{{(.exe)?}}" "-mabi" "lp64" "-march" "rv64g"
Index: lib/Driver/ToolChains/Gnu.cpp
===
--- lib/Driver/ToolChains/Gnu.cpp
+++ lib/Driver/ToolChains/Gnu.cpp
@@ -629,6 +629,18 @@
   ppc::getPPCAsmModeForCPU(getCPUName(Args, getToolChain().getTriple(;
 break;
   }
+  case llvm::Triple::riscv32:
+  case llvm::Triple::riscv64: {
+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());
+}
+break;
+  }
   case llvm::Triple::sparc:
   case llvm::Triple::sparcel: {
 CmdArgs.push_back("-32");
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D42666: Revert "Revert rC322769: [RISCV] Propagate -mabi and -march values to GNU assembler."

[Ana Pazos via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL323894: Revert Revert rC322769: [RISCV] Propagate 
-mabi and -march values to GNU… (authored by apazos, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D42666?vs=131886=132207#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D42666

Files:
  cfe/trunk/lib/Driver/ToolChains/Gnu.cpp
  cfe/trunk/test/Driver/riscv-gnutools.c


Index: cfe/trunk/test/Driver/riscv-gnutools.c
===
--- cfe/trunk/test/Driver/riscv-gnutools.c
+++ cfe/trunk/test/Driver/riscv-gnutools.c
@@ -0,0 +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: %clang -target riscv32 -fno-integrated-as -march=rv32g %s -### -c \
+// RUN: 2>&1 | FileCheck -check-prefix=MABI-ILP32-MARCH-G %s
+
+// RUN: %clang -target riscv64 -fno-integrated-as %s -###  -c \
+// RUN: 2>&1 | FileCheck -check-prefix=MABI-ILP64 %s
+
+// RUN: %clang -target riscv64 -fno-integrated-as -march=rv64g %s -### -c \
+// RUN: 2>&1 | FileCheck -check-prefix=MABI-ILP64-MARCH-G %s
+
+// MABI-ILP32: "{{.*}}as{{(.exe)?}}" "-mabi" "ilp32"
+// MABI-ILP32-MARCH-G: "{{.*}}as{{(.exe)?}}" "-mabi" "ilp32" "-march" "rv32g"
+
+// MABI-ILP64: "{{.*}}as{{(.exe)?}}" "-mabi" "lp64"
+// MABI-ILP64-MARCH-G: "{{.*}}as{{(.exe)?}}" "-mabi" "lp64" "-march" "rv64g"
Index: cfe/trunk/lib/Driver/ToolChains/Gnu.cpp
===
--- cfe/trunk/lib/Driver/ToolChains/Gnu.cpp
+++ cfe/trunk/lib/Driver/ToolChains/Gnu.cpp
@@ -629,6 +629,18 @@
   ppc::getPPCAsmModeForCPU(getCPUName(Args, getToolChain().getTriple(;
 break;
   }
+  case llvm::Triple::riscv32:
+  case llvm::Triple::riscv64: {
+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());
+}
+break;
+  }
   case llvm::Triple::sparc:
   case llvm::Triple::sparcel: {
 CmdArgs.push_back("-32");


Index: cfe/trunk/test/Driver/riscv-gnutools.c
===
--- cfe/trunk/test/Driver/riscv-gnutools.c
+++ cfe/trunk/test/Driver/riscv-gnutools.c
@@ -0,0 +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: %clang -target riscv32 -fno-integrated-as -march=rv32g %s -### -c \
+// RUN: 2>&1 | FileCheck -check-prefix=MABI-ILP32-MARCH-G %s
+
+// RUN: %clang -target riscv64 -fno-integrated-as %s -###  -c \
+// RUN: 2>&1 | FileCheck -check-prefix=MABI-ILP64 %s
+
+// RUN: %clang -target riscv64 -fno-integrated-as -march=rv64g %s -### -c \
+// RUN: 2>&1 | FileCheck -check-prefix=MABI-ILP64-MARCH-G %s
+
+// MABI-ILP32: "{{.*}}as{{(.exe)?}}" "-mabi" "ilp32"
+// MABI-ILP32-MARCH-G: "{{.*}}as{{(.exe)?}}" "-mabi" "ilp32" "-march" "rv32g"
+
+// MABI-ILP64: "{{.*}}as{{(.exe)?}}" "-mabi" "lp64"
+// MABI-ILP64-MARCH-G: "{{.*}}as{{(.exe)?}}" "-mabi" "lp64" "-march" "rv64g"
Index: cfe/trunk/lib/Driver/ToolChains/Gnu.cpp
===
--- cfe/trunk/lib/Driver/ToolChains/Gnu.cpp
+++ cfe/trunk/lib/Driver/ToolChains/Gnu.cpp
@@ -629,6 +629,18 @@
   ppc::getPPCAsmModeForCPU(getCPUName(Args, getToolChain().getTriple(;
 break;
   }
+  case llvm::Triple::riscv32:
+  case llvm::Triple::riscv64: {
+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());
+}
+break;
+  }
   case llvm::Triple::sparc:
   case llvm::Triple::sparcel: {
 CmdArgs.push_back("-32");
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D41271: [RISCV] Propagate -mabi and -march values to GNU assembler.

[Ana Pazos via Phabricator via cfe-commits]

apazos added a comment.

Committed R322769


Repository:
  rL LLVM

https://reviews.llvm.org/D41271



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

[PATCH] D41271: [RISCV] Propagate -mabi and -march values to GNU assembler.

[Ana Pazos via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL322769: [RISCV] Propagate -mabi and -march values to GNU 
assembler. (authored by apazos, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D41271?vs=129981=130272#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D41271

Files:
  cfe/trunk/lib/Driver/ToolChains/Gnu.cpp
  
cfe/trunk/test/Driver/Inputs/multilib_riscv_linux_sdk/riscv64-unknown-linux-gnu/bin/as
  cfe/trunk/test/Driver/riscv-gnutools.c


Index: 
cfe/trunk/test/Driver/Inputs/multilib_riscv_linux_sdk/riscv64-unknown-linux-gnu/bin/as
===
--- 
cfe/trunk/test/Driver/Inputs/multilib_riscv_linux_sdk/riscv64-unknown-linux-gnu/bin/as
+++ 
cfe/trunk/test/Driver/Inputs/multilib_riscv_linux_sdk/riscv64-unknown-linux-gnu/bin/as
@@ -0,0 +1 @@
+#!/bin/true
Index: cfe/trunk/test/Driver/riscv-gnutools.c
===
--- cfe/trunk/test/Driver/riscv-gnutools.c
+++ cfe/trunk/test/Driver/riscv-gnutools.c
@@ -0,0 +1,14 @@
+// Check gnutools are invoked with propagated values for -mabi and -march.
+
+// RUN: %clang -target riscv32-linux-unknown-elf -fno-integrated-as \
+// RUN: --gcc-toolchain=%S/Inputs/multilib_riscv_linux_sdk \
+// RUN: --sysroot=%S/Inputs/multilib_riscv_linux_sdk/sysroot %s -### \
+// RUN: 2>&1 | FileCheck -check-prefix=MABI-ILP32 %s
+// RUN: %clang -target riscv32-linux-unknown-elf -fno-integrated-as \
+// RUN: -march=rv32g --gcc-toolchain=%S/Inputs/multilib_riscv_linux_sdk \
+// RUN: --sysroot=%S/Inputs/multilib_riscv_linux_sdk/sysroot %s -### \
+// RUN: 2>&1 | FileCheck -check-prefix=MABI-ILP32-MARCH-G %s
+
+// MABI-ILP32: 
"{{.*}}/Inputs/multilib_riscv_linux_sdk/lib/gcc/riscv64-unknown-linux-gnu/7.2.0/../../../../riscv64-unknown-linux-gnu/bin{{/|}}as"
 "-mabi" "ilp32"
+// MABI-ILP32-MARCH-G: 
"{{.*}}/Inputs/multilib_riscv_linux_sdk/lib/gcc/riscv64-unknown-linux-gnu/7.2.0/../../../../riscv64-unknown-linux-gnu/bin{{/|}}as"
 "-mabi" "ilp32" "-march" "rv32g"
+
Index: cfe/trunk/lib/Driver/ToolChains/Gnu.cpp
===
--- cfe/trunk/lib/Driver/ToolChains/Gnu.cpp
+++ cfe/trunk/lib/Driver/ToolChains/Gnu.cpp
@@ -629,6 +629,18 @@
   ppc::getPPCAsmModeForCPU(getCPUName(Args, getToolChain().getTriple(;
 break;
   }
+  case llvm::Triple::riscv32:
+  case llvm::Triple::riscv64: {
+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());
+}
+break;
+  }
   case llvm::Triple::sparc:
   case llvm::Triple::sparcel: {
 CmdArgs.push_back("-32");


Index: cfe/trunk/test/Driver/Inputs/multilib_riscv_linux_sdk/riscv64-unknown-linux-gnu/bin/as
===
--- cfe/trunk/test/Driver/Inputs/multilib_riscv_linux_sdk/riscv64-unknown-linux-gnu/bin/as
+++ cfe/trunk/test/Driver/Inputs/multilib_riscv_linux_sdk/riscv64-unknown-linux-gnu/bin/as
@@ -0,0 +1 @@
+#!/bin/true
Index: cfe/trunk/test/Driver/riscv-gnutools.c
===
--- cfe/trunk/test/Driver/riscv-gnutools.c
+++ cfe/trunk/test/Driver/riscv-gnutools.c
@@ -0,0 +1,14 @@
+// Check gnutools are invoked with propagated values for -mabi and -march.
+
+// RUN: %clang -target riscv32-linux-unknown-elf -fno-integrated-as \
+// RUN: --gcc-toolchain=%S/Inputs/multilib_riscv_linux_sdk \
+// RUN: --sysroot=%S/Inputs/multilib_riscv_linux_sdk/sysroot %s -### \
+// RUN: 2>&1 | FileCheck -check-prefix=MABI-ILP32 %s
+// RUN: %clang -target riscv32-linux-unknown-elf -fno-integrated-as \
+// RUN: -march=rv32g --gcc-toolchain=%S/Inputs/multilib_riscv_linux_sdk \
+// RUN: --sysroot=%S/Inputs/multilib_riscv_linux_sdk/sysroot %s -### \
+// RUN: 2>&1 | FileCheck -check-prefix=MABI-ILP32-MARCH-G %s
+
+// MABI-ILP32: "{{.*}}/Inputs/multilib_riscv_linux_sdk/lib/gcc/riscv64-unknown-linux-gnu/7.2.0/../../../../riscv64-unknown-linux-gnu/bin{{/|}}as" "-mabi" "ilp32"
+// MABI-ILP32-MARCH-G: "{{.*}}/Inputs/multilib_riscv_linux_sdk/lib/gcc/riscv64-unknown-linux-gnu/7.2.0/../../../../riscv64-unknown-linux-gnu/bin{{/|}}as" "-mabi" "ilp32" "-march" "rv32g"
+
Index: cfe/trunk/lib/Driver/ToolChains/Gnu.cpp
===
--- cfe/trunk/lib/Driver/ToolChains/Gnu.cpp
+++ cfe/trunk/lib/Driver/ToolChains/Gnu.cpp
@@ -629,6 +629,18 @@
   ppc::getPPCAsmModeForCPU(getCPUName(Args, getToolChain().getTriple(;
 break;
   }
+  case llvm::Triple::riscv32:
+  case llvm::Triple::riscv64: {
+StringRef ABIName = riscv::getRISCVABI(Args, getToolChain().getTriple());
+CmdArgs.push_back("-mabi");
+ 

[PATCH] D41271: [RISCV] Propagate -mabi and -march values to GNU assembler.

[Ana Pazos via Phabricator via cfe-commits]

apazos added a comment.

Thanks for the clarifications Alex. 
No need to fix  these known issues right now.
I will proceed with checking in this change.


https://reviews.llvm.org/D41271



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

[PATCH] D41271: [RISCV] Propagate -mabi and -march values to GNU assembler.

[Ana Pazos via Phabricator via cfe-commits]

apazos updated this revision to Diff 129981.
apazos added a comment.

I tested this on windows and I had to add an assembler placeholder executable, 
just like it was done with the linker in the RISCV multilib dir checked under 
Inputs. 
Other observations, are these known issues?

- multilib dir checked in has only riscv64-unknown-linux-gnu which we see the 
riscv tests invoking even when the target is 32 bit.
- riscv64 is not honoring gcc-toolchain flag, it keeps invoking default 
/usr/bin/as.

Anyways, the test now passes on windows.


https://reviews.llvm.org/D41271

Files:
  lib/Driver/ToolChains/Gnu.cpp
  test/Driver/Inputs/multilib_riscv_linux_sdk/riscv64-unknown-linux-gnu/bin/as
  test/Driver/riscv-gnutools.c


Index: test/Driver/riscv-gnutools.c
===
--- /dev/null
+++ test/Driver/riscv-gnutools.c
@@ -0,0 +1,14 @@
+// Check gnutools are invoked with propagated values for -mabi and -march.
+
+// RUN: %clang -target riscv32-linux-unknown-elf -fno-integrated-as \
+// RUN: --gcc-toolchain=%S/Inputs/multilib_riscv_linux_sdk \
+// RUN: --sysroot=%S/Inputs/multilib_riscv_linux_sdk/sysroot %s -### \
+// RUN: 2>&1 | FileCheck -check-prefix=MABI-ILP32 %s
+// RUN: %clang -target riscv32-linux-unknown-elf -fno-integrated-as \
+// RUN: -march=rv32g --gcc-toolchain=%S/Inputs/multilib_riscv_linux_sdk \
+// RUN: --sysroot=%S/Inputs/multilib_riscv_linux_sdk/sysroot %s -### \
+// RUN: 2>&1 | FileCheck -check-prefix=MABI-ILP32-MARCH-G %s
+
+// MABI-ILP32: 
"{{.*}}/Inputs/multilib_riscv_linux_sdk/lib/gcc/riscv64-unknown-linux-gnu/7.2.0/../../../../riscv64-unknown-linux-gnu/bin{{/|}}as"
 "-mabi" "ilp32"
+// MABI-ILP32-MARCH-G: 
"{{.*}}/Inputs/multilib_riscv_linux_sdk/lib/gcc/riscv64-unknown-linux-gnu/7.2.0/../../../../riscv64-unknown-linux-gnu/bin{{/|}}as"
 "-mabi" "ilp32" "-march" "rv32g"
+
Index: 
test/Driver/Inputs/multilib_riscv_linux_sdk/riscv64-unknown-linux-gnu/bin/as
===
--- /dev/null
+++ test/Driver/Inputs/multilib_riscv_linux_sdk/riscv64-unknown-linux-gnu/bin/as
@@ -0,0 +1 @@
+#!/bin/true
Index: lib/Driver/ToolChains/Gnu.cpp
===
--- lib/Driver/ToolChains/Gnu.cpp
+++ lib/Driver/ToolChains/Gnu.cpp
@@ -629,6 +629,18 @@
   ppc::getPPCAsmModeForCPU(getCPUName(Args, getToolChain().getTriple(;
 break;
   }
+  case llvm::Triple::riscv32:
+  case llvm::Triple::riscv64: {
+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());
+}
+break;
+  }
   case llvm::Triple::sparc:
   case llvm::Triple::sparcel: {
 CmdArgs.push_back("-32");


Index: test/Driver/riscv-gnutools.c
===
--- /dev/null
+++ test/Driver/riscv-gnutools.c
@@ -0,0 +1,14 @@
+// Check gnutools are invoked with propagated values for -mabi and -march.
+
+// RUN: %clang -target riscv32-linux-unknown-elf -fno-integrated-as \
+// RUN: --gcc-toolchain=%S/Inputs/multilib_riscv_linux_sdk \
+// RUN: --sysroot=%S/Inputs/multilib_riscv_linux_sdk/sysroot %s -### \
+// RUN: 2>&1 | FileCheck -check-prefix=MABI-ILP32 %s
+// RUN: %clang -target riscv32-linux-unknown-elf -fno-integrated-as \
+// RUN: -march=rv32g --gcc-toolchain=%S/Inputs/multilib_riscv_linux_sdk \
+// RUN: --sysroot=%S/Inputs/multilib_riscv_linux_sdk/sysroot %s -### \
+// RUN: 2>&1 | FileCheck -check-prefix=MABI-ILP32-MARCH-G %s
+
+// MABI-ILP32: "{{.*}}/Inputs/multilib_riscv_linux_sdk/lib/gcc/riscv64-unknown-linux-gnu/7.2.0/../../../../riscv64-unknown-linux-gnu/bin{{/|}}as" "-mabi" "ilp32"
+// MABI-ILP32-MARCH-G: "{{.*}}/Inputs/multilib_riscv_linux_sdk/lib/gcc/riscv64-unknown-linux-gnu/7.2.0/../../../../riscv64-unknown-linux-gnu/bin{{/|}}as" "-mabi" "ilp32" "-march" "rv32g"
+
Index: test/Driver/Inputs/multilib_riscv_linux_sdk/riscv64-unknown-linux-gnu/bin/as
===
--- /dev/null
+++ test/Driver/Inputs/multilib_riscv_linux_sdk/riscv64-unknown-linux-gnu/bin/as
@@ -0,0 +1 @@
+#!/bin/true
Index: lib/Driver/ToolChains/Gnu.cpp
===
--- lib/Driver/ToolChains/Gnu.cpp
+++ lib/Driver/ToolChains/Gnu.cpp
@@ -629,6 +629,18 @@
   ppc::getPPCAsmModeForCPU(getCPUName(Args, getToolChain().getTriple(;
 break;
   }
+  case llvm::Triple::riscv32:
+  case llvm::Triple::riscv64: {
+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();

[PATCH] D41271: [RISCV] Propagate -mabi and -march values to GNU assembler.

[Ana Pazos via Phabricator via cfe-commits]

apazos added a comment.

thanks Alex, I will test on windows before pushing.


https://reviews.llvm.org/D41271



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

[PATCH] D41271: [RISCV] Propagate -mabi and -march values to GNU assembler.

[Ana Pazos via Phabricator via cfe-commits]

apazos updated this revision to Diff 129476.
apazos added a comment.
Herald added a subscriber: niosHD.

Rebased.


https://reviews.llvm.org/D41271

Files:
  lib/Driver/ToolChains/Gnu.cpp
  test/Driver/riscv-gnutools.c


Index: test/Driver/riscv-gnutools.c
===
--- /dev/null
+++ test/Driver/riscv-gnutools.c
@@ -0,0 +1,18 @@
+// Check gnutools are invoked with propagated values for -mabi and -march.
+
+// RUN: %clang -target riscv32-unknown-elf -fno-integrated-as %s -### \
+// RUN: 2>&1 | FileCheck -check-prefix=MABI-ILP32 %s
+// RUN: %clang -target riscv64-unknown-elf -fno-integrated-as %s -### \
+// RUN: 2>&1 | FileCheck -check-prefix=MABI-LP64 %s
+
+// MABI-ILP32: "/usr/bin/as" "-mabi" "ilp32"
+// MABI-LP64: "/usr/bin/as" "-mabi" "lp64"
+
+// RUN: %clang -target riscv32-unknown-elf -fno-integrated-as -march=rv32g %s 
-### \
+// RUN: 2>&1 | FileCheck -check-prefix=MABI-ILP32-MARCH-G %s
+// RUN: %clang -target riscv64-unknown-elf -fno-integrated-as -march=rv64g %s 
-### \
+// RUN: 2>&1 | FileCheck -check-prefix=MABI-LP64-MARCH-G %s
+
+// MABI-ILP32-MARCH-G: "/usr/bin/as" "-mabi" "ilp32" "-march" "rv32g"
+// MABI-LP64-MARCH-G: "/usr/bin/as" "-mabi" "lp64" "-march" "rv64g"
+
Index: lib/Driver/ToolChains/Gnu.cpp
===
--- lib/Driver/ToolChains/Gnu.cpp
+++ lib/Driver/ToolChains/Gnu.cpp
@@ -629,6 +629,18 @@
   ppc::getPPCAsmModeForCPU(getCPUName(Args, getToolChain().getTriple(;
 break;
   }
+  case llvm::Triple::riscv32:
+  case llvm::Triple::riscv64: {
+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());
+}
+break;
+  }
   case llvm::Triple::sparc:
   case llvm::Triple::sparcel: {
 CmdArgs.push_back("-32");


Index: test/Driver/riscv-gnutools.c
===
--- /dev/null
+++ test/Driver/riscv-gnutools.c
@@ -0,0 +1,18 @@
+// Check gnutools are invoked with propagated values for -mabi and -march.
+
+// RUN: %clang -target riscv32-unknown-elf -fno-integrated-as %s -### \
+// RUN: 2>&1 | FileCheck -check-prefix=MABI-ILP32 %s
+// RUN: %clang -target riscv64-unknown-elf -fno-integrated-as %s -### \
+// RUN: 2>&1 | FileCheck -check-prefix=MABI-LP64 %s
+
+// MABI-ILP32: "/usr/bin/as" "-mabi" "ilp32"
+// MABI-LP64: "/usr/bin/as" "-mabi" "lp64"
+
+// RUN: %clang -target riscv32-unknown-elf -fno-integrated-as -march=rv32g %s -### \
+// RUN: 2>&1 | FileCheck -check-prefix=MABI-ILP32-MARCH-G %s
+// RUN: %clang -target riscv64-unknown-elf -fno-integrated-as -march=rv64g %s -### \
+// RUN: 2>&1 | FileCheck -check-prefix=MABI-LP64-MARCH-G %s
+
+// MABI-ILP32-MARCH-G: "/usr/bin/as" "-mabi" "ilp32" "-march" "rv32g"
+// MABI-LP64-MARCH-G: "/usr/bin/as" "-mabi" "lp64" "-march" "rv64g"
+
Index: lib/Driver/ToolChains/Gnu.cpp
===
--- lib/Driver/ToolChains/Gnu.cpp
+++ lib/Driver/ToolChains/Gnu.cpp
@@ -629,6 +629,18 @@
   ppc::getPPCAsmModeForCPU(getCPUName(Args, getToolChain().getTriple(;
 break;
   }
+  case llvm::Triple::riscv32:
+  case llvm::Triple::riscv64: {
+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());
+}
+break;
+  }
   case llvm::Triple::sparc:
   case llvm::Triple::sparcel: {
 CmdArgs.push_back("-32");
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D41271: [RISCV] Propagate -mabi and -march values to GNU assembler.

[Ana Pazos via Phabricator via cfe-commits]

apazos added a comment.

This is ready to merge, just waiting for the dependence 
https://reviews.llvm.org/D39963 to be merged first.


https://reviews.llvm.org/D41271



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

[PATCH] D40023: [RISCV] Implement ABI lowering

[Ana Pazos via Phabricator via cfe-commits]

apazos added a comment.

Hi Alex, just a reminder, it looks like Eli's and David's comments have not 
been addressed yet.


https://reviews.llvm.org/D40023



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

[PATCH] D39963: [RISCV] Add initial RISC-V target and driver support

[Ana Pazos via Phabricator via cfe-commits]

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

Please merge this patch, it looks in good shape. This patch is required for any 
RISCV build.


https://reviews.llvm.org/D39963



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

[PATCH] D41271: [RISCV] Propagate -mabi and -march values to GNU assembler.

[Ana Pazos via Phabricator via cfe-commits]

apazos updated this revision to Diff 127170.
apazos added a comment.

changed label prefix ILP64 to LP64


https://reviews.llvm.org/D41271

Files:
  lib/Driver/ToolChains/Gnu.cpp
  test/Driver/riscv-gnutools.c


Index: test/Driver/riscv-gnutools.c
===
--- /dev/null
+++ test/Driver/riscv-gnutools.c
@@ -0,0 +1,18 @@
+// Check gnutools are invoked with propagated values for -mabi and -march.
+
+// RUN: %clang -target riscv32-unknown-elf -fno-integrated-as %s -### \
+// RUN: 2>&1 | FileCheck -check-prefix=MABI-ILP32 %s
+// RUN: %clang -target riscv64-unknown-elf -fno-integrated-as %s -### \
+// RUN: 2>&1 | FileCheck -check-prefix=MABI-LP64 %s
+
+// MABI-ILP32: "/usr/bin/as" "-mabi" "ilp32"
+// MABI-LP64: "/usr/bin/as" "-mabi" "lp64"
+
+// RUN: %clang -target riscv32-unknown-elf -fno-integrated-as -march=rv32g %s 
-### \
+// RUN: 2>&1 | FileCheck -check-prefix=MABI-ILP32-MARCH-G %s
+// RUN: %clang -target riscv64-unknown-elf -fno-integrated-as -march=rv64g %s 
-### \
+// RUN: 2>&1 | FileCheck -check-prefix=MABI-LP64-MARCH-G %s
+
+// MABI-ILP32-MARCH-G: "/usr/bin/as" "-mabi" "ilp32" "-march" "rv32g"
+// MABI-LP64-MARCH-G: "/usr/bin/as" "-mabi" "lp64" "-march" "rv64g"
+
Index: lib/Driver/ToolChains/Gnu.cpp
===
--- lib/Driver/ToolChains/Gnu.cpp
+++ lib/Driver/ToolChains/Gnu.cpp
@@ -629,6 +629,18 @@
   ppc::getPPCAsmModeForCPU(getCPUName(Args, getToolChain().getTriple(;
 break;
   }
+  case llvm::Triple::riscv32:
+  case llvm::Triple::riscv64: {
+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());
+}
+break;
+  }
   case llvm::Triple::sparc:
   case llvm::Triple::sparcel: {
 CmdArgs.push_back("-32");


Index: test/Driver/riscv-gnutools.c
===
--- /dev/null
+++ test/Driver/riscv-gnutools.c
@@ -0,0 +1,18 @@
+// Check gnutools are invoked with propagated values for -mabi and -march.
+
+// RUN: %clang -target riscv32-unknown-elf -fno-integrated-as %s -### \
+// RUN: 2>&1 | FileCheck -check-prefix=MABI-ILP32 %s
+// RUN: %clang -target riscv64-unknown-elf -fno-integrated-as %s -### \
+// RUN: 2>&1 | FileCheck -check-prefix=MABI-LP64 %s
+
+// MABI-ILP32: "/usr/bin/as" "-mabi" "ilp32"
+// MABI-LP64: "/usr/bin/as" "-mabi" "lp64"
+
+// RUN: %clang -target riscv32-unknown-elf -fno-integrated-as -march=rv32g %s -### \
+// RUN: 2>&1 | FileCheck -check-prefix=MABI-ILP32-MARCH-G %s
+// RUN: %clang -target riscv64-unknown-elf -fno-integrated-as -march=rv64g %s -### \
+// RUN: 2>&1 | FileCheck -check-prefix=MABI-LP64-MARCH-G %s
+
+// MABI-ILP32-MARCH-G: "/usr/bin/as" "-mabi" "ilp32" "-march" "rv32g"
+// MABI-LP64-MARCH-G: "/usr/bin/as" "-mabi" "lp64" "-march" "rv64g"
+
Index: lib/Driver/ToolChains/Gnu.cpp
===
--- lib/Driver/ToolChains/Gnu.cpp
+++ lib/Driver/ToolChains/Gnu.cpp
@@ -629,6 +629,18 @@
   ppc::getPPCAsmModeForCPU(getCPUName(Args, getToolChain().getTriple(;
 break;
   }
+  case llvm::Triple::riscv32:
+  case llvm::Triple::riscv64: {
+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());
+}
+break;
+  }
   case llvm::Triple::sparc:
   case llvm::Triple::sparcel: {
 CmdArgs.push_back("-32");
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D40023: [RISCV] Implement ABI lowering

[Ana Pazos via Phabricator via cfe-commits]

apazos added inline comments.



Comment at: lib/CodeGen/TargetInfo.cpp:8872
+  else
+NeededArgGPRs = 1;
+

Suggestion to make 1 default value when you declare the var



Comment at: lib/CodeGen/TargetInfo.cpp:8938
+
+  // The size of the actual thing passed, which might end up just
+  // being a pointer for indirect types.

The size -> The type


https://reviews.llvm.org/D40023



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

[PATCH] D39963: [RISCV][RFC] Add initial RISC-V target and driver support

[Ana Pazos via Phabricator via cfe-commits]

apazos added a comment.

Can you push this as a patch to review/commit instead of RFC? It has received a 
lot of comments/corrections already and I think it is getting in a shape we can 
merge.




Comment at: lib/Driver/ToolChains/RISCV.cpp:86
+CmdArgs.push_back("-lc");
+CmdArgs.push_back("-lgloss");
+CmdArgs.push_back("--end-group");

asb wrote:
> apazos wrote:
> > mgrang wrote:
> > > How about if our sysroot is linux (as opposed to elf)? There won't be any 
> > > libgloss.a, right? Also there won't be a crt0.o (instead there will be 
> > > crt1.o).
> > Supporting linux target is desirable early on because most of us will be 
> > using Qemu for running tests.
> Linux targets are not currently supported, as they require the ilp32d or 
> lp64d (hard double precision) ABI. The only fiddly part is actually in the 
> Clang frontend, handling structs composed of two reals or one integer + one 
> real. This is obviously high up on the todo list.
I meant user code compiled with target=riscv32-unknown-linux march=rv32imafdc 
and mabi=ilp32. This would run fine on qemu.



Comment at: test/Preprocessor/init.c:9991
+// RUN: %clang_cc1 -E -dM -ffreestanding -triple=riscv32 < /dev/null \
+// RUN:   | FileCheck -match-full-lines -check-prefix=RISCV32 %s
+// RISCV32: #define _ILP32 1

Shouldn't we just check for the target specific defines  in these tests?


https://reviews.llvm.org/D39963



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

[PATCH] D39963: [RISCV][RFC] Add initial RISC-V target and driver support

[Ana Pazos via Phabricator via cfe-commits]

apazos added inline comments.



Comment at: lib/Driver/ToolChains/RISCV.cpp:86
+CmdArgs.push_back("-lc");
+CmdArgs.push_back("-lgloss");
+CmdArgs.push_back("--end-group");

mgrang wrote:
> How about if our sysroot is linux (as opposed to elf)? There won't be any 
> libgloss.a, right? Also there won't be a crt0.o (instead there will be 
> crt1.o).
Supporting linux target is desirable early on because most of us will be using 
Qemu for running tests.


https://reviews.llvm.org/D39963



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

[PATCH] D34425: Unified ARM logic for computing target ABI.

[Ana Pazos via Phabricator via cfe-commits]

apazos created this revision.
Herald added subscribers: kristof.beyls, javed.absar, srhines, aemerson.

Unified logic for computing target ABI in backend and front end by moving this 
common code to Support/TargetParser.

Modeled Triple::GNU after front end code (aapcs abi) and  updated tests that 
expect apcs abi.

Removed assertion in backend to avoid different compiler behavior due to build 
mode.


https://reviews.llvm.org/D34425

Files:
  lib/Driver/ToolChains/Clang.cpp
  test/CodeGen/arm-v8.1a-neon-intrinsics.c
  test/CodeGen/named_reg_global.c
  test/CodeGen/neon-immediate-ubsan.c
  test/CodeGen/xray-attributes-supported.cpp

Index: test/CodeGen/xray-attributes-supported.cpp
===
--- test/CodeGen/xray-attributes-supported.cpp
+++ test/CodeGen/xray-attributes-supported.cpp
@@ -1,5 +1,5 @@
 // RUN: %clang_cc1 %s -fxray-instrument -std=c++11 -x c++ -emit-llvm -o - -triple x86_64-unknown-linux-gnu | FileCheck %s
-// RUN: %clang_cc1 %s -fxray-instrument -std=c++11 -x c++ -emit-llvm -o - -triple arm-unknown-linux-gnu | FileCheck %s
+// RUN: %clang_cc1 %s -fxray-instrument -std=c++11 -x c++ -emit-llvm -o - -triple arm-unknown-linux-gnu -target-abi apcs-gnu | FileCheck %s
 // RUN: %clang_cc1 %s -fxray-instrument -std=c++11 -x c++ -emit-llvm -o - -triple mips-unknown-linux-gnu | FileCheck %s
 // RUN: %clang_cc1 %s -fxray-instrument -std=c++11 -x c++ -emit-llvm -o - -triple mipsel-unknown-linux-gnu | FileCheck %s
 // RUN: %clang_cc1 %s -fxray-instrument -std=c++11 -x c++ -emit-llvm -o - -triple mips64-unknown-linux-gnu | FileCheck %s
Index: test/CodeGen/neon-immediate-ubsan.c
===
--- test/CodeGen/neon-immediate-ubsan.c
+++ test/CodeGen/neon-immediate-ubsan.c
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -triple armv7s-linux-gnu -emit-llvm -o - %s \
+// RUN: %clang_cc1 -triple armv7s-linux-gnu -target-abi apcs-gnu -emit-llvm -o - %s \
 // RUN: -target-feature +neon -target-cpu cortex-a8 \
 // RUN: -fsanitize=signed-integer-overflow \
 // RUN:   | FileCheck %s --check-prefix=CHECK --check-prefix=ARMV7
Index: test/CodeGen/named_reg_global.c
===
--- test/CodeGen/named_reg_global.c
+++ test/CodeGen/named_reg_global.c
@@ -1,6 +1,6 @@
 // RUN: %clang_cc1 -triple x86_64-linux-gnu -S -emit-llvm %s -o - | FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-X86-64
 // RUN: %clang_cc1 -triple arm64-linux-gnu -S -emit-llvm %s -o - | FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-ARM
-// RUN: %clang_cc1 -triple armv7-linux-gnu -S -emit-llvm %s -o - | FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-ARM
+// RUN: %clang_cc1 -triple armv7-linux-gnu -target-abi apcs-gnu -S -emit-llvm %s -o - | FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-ARM
 
 // CHECK-NOT: @sp = common global
 
Index: test/CodeGen/arm-v8.1a-neon-intrinsics.c
===
--- test/CodeGen/arm-v8.1a-neon-intrinsics.c
+++ test/CodeGen/arm-v8.1a-neon-intrinsics.c
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -triple armv8.1a-linux-gnu -target-feature +neon \
+// RUN: %clang_cc1 -triple armv8.1a-linux-gnu -target-abi apcs-gnu -target-feature +neon \
 // RUN:  -S -emit-llvm -o - %s \
 // RUN:  | FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-ARM
 
Index: lib/Driver/ToolChains/Clang.cpp
===
--- lib/Driver/ToolChains/Clang.cpp
+++ lib/Driver/ToolChains/Clang.cpp
@@ -35,6 +35,7 @@
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/Process.h"
+#include "llvm/Support/TargetParser.h"
 #include "llvm/Support/YAMLParser.h"
 
 #ifdef LLVM_ON_UNIX
@@ -1278,43 +1279,13 @@
   // FIXME: Support -meabi.
   // FIXME: Parts of this are duplicated in the backend, unify this somehow.
   const char *ABIName = nullptr;
-  if (Arg *A = Args.getLastArg(options::OPT_mabi_EQ)) {
+  if (Arg *A = Args.getLastArg(options::OPT_mabi_EQ))
 ABIName = A->getValue();
-  } else if (Triple.isOSBinFormatMachO()) {
-if (arm::useAAPCSForMachO(Triple)) {
-  ABIName = "aapcs";
-} else if (Triple.isWatchABI()) {
-  ABIName = "aapcs16";
-} else {
-  ABIName = "apcs-gnu";
-}
-  } else if (Triple.isOSWindows()) {
-// FIXME: this is invalid for WindowsCE
-ABIName = "aapcs";
-  } else {
-// Select the default based on the platform.
-switch (Triple.getEnvironment()) {
-case llvm::Triple::Android:
-case llvm::Triple::GNUEABI:
-case llvm::Triple::GNUEABIHF:
-case llvm::Triple::MuslEABI:
-case llvm::Triple::MuslEABIHF:
-  ABIName = "aapcs-linux";
-  break;
-case llvm::Triple::EABIHF:
-case llvm::Triple::EABI:
-  ABIName = "aapcs";
-  break;
-default:
-  if (Triple.getOS() == llvm::Triple::NetBSD)
-ABIName = "apcs-gnu";
-  else if