[PATCH] D70401: [RISCV] CodeGen of RVE and ilp32e/lp64e ABIs

[Wang Pengcheng via Phabricator via cfe-commits]

wangpc added a comment.

In D70401#4657101 , @jrtc27 wrote:

> In D70401#4657098 , @jrtc27 wrote:
>
>> GCC only ever defines __riscv_32e
>
> Hm, seems the comments about __riscv_32e were from months ago, ignore them if 
> they aren't correct or have become outdated...

FYI: https://github.com/riscv-non-isa/riscv-c-api-doc/pull/52


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70401

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

[PATCH] D70401: [RISCV] CodeGen of RVE and ilp32e/lp64e ABIs

[Jessica Clarke via Phabricator via cfe-commits]

jrtc27 added a comment.

In D70401#4657098 , @jrtc27 wrote:

> GCC only ever defines __riscv_32e

Hm, seems the comments about __riscv_32e were from months ago, ignore them if 
they aren't correct or have become outdated...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70401

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

[PATCH] D70401: [RISCV] CodeGen of RVE and ilp32e/lp64e ABIs

[Jessica Clarke via Phabricator via cfe-commits]

jrtc27 added a comment.

GCC only ever defines __riscv_32e




Comment at: clang/lib/Basic/Targets/RISCV.cpp:210
+if (Is64Bit)
+  Builder.defineMacro("__riscv_64e");
+else

Ugh, these don't align with the normal pattern. __riscv_e already exists in the 
spec, can we just leave __riscv_32e as deprecated for RV32E and not introduce 
the old-style __riscv_64e?



Comment at: clang/lib/Basic/Targets/RISCV.h:139
 if (Name == "ilp32" || Name == "ilp32f" || Name == "ilp32d") {
   ABI = Name;
   return true;

Does it matter we don't undo the effects of the RVE ABI here?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70401

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

[PATCH] D70401: [RISCV] CodeGen of RVE and ilp32e/lp64e ABIs

[Wang Pengcheng via Phabricator via cfe-commits]

wangpc added a comment.

@craig.topper Thanks!
@asb Hi Alex, I'd like to get another approval from you. Are there any more 
concerns?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70401

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

[PATCH] D70401: [RISCV] CodeGen of RVE and ilp32e/lp64e ABIs

[Craig Topper via Phabricator via cfe-commits]

craig.topper accepted this revision.
craig.topper added a comment.

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70401

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

[PATCH] D70401: [RISCV] CodeGen of RVE and ilp32e/lp64e ABIs

[Koute via Phabricator via cfe-commits]

koute added a comment.

Sorry for the comment spam, but could we please get this merged in finally? (:

To people who hold the decision making power as to whether this is merged: are 
there still any blockers left, considering the consensus was to merge it? 
What's the hold up? Is there anything I can do to help?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70401

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

[PATCH] D70401: [RISCV] CodeGen of RVE and ilp32e/lp64e ABIs

[Alex Bradbury via Phabricator via cfe-commits]

asb added a comment.

Thanks, I'll take another look. Rereading my previous comment I just wanted to 
clarify one part so it's not misunderstood. I said " I think it's unfortunate 
that this need for the ABI hasn't translated into effort to finalise the ABI 
definition in the psABI doc and to at least get it to match what GCC actually 
implements" - I wanted to be __very__ clear this isn't a criticism of those 
trying to maintain the ABI doc, it's about companies who want to ship RVE 
hardware and software not contributing to that process.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70401

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

[PATCH] D70401: [RISCV] CodeGen of RVE and ilp32e/lp64e ABIs

[Wang Pengcheng via Phabricator via cfe-commits]

wangpc added a comment.

In D70401#4655408 , @asb wrote:

> First of all, thank you to everyone who has been trying to nudge this forward 
> and apologies it must have been a frustrating experience.
>
> I appreciate there are users who want to see this and I don't like that LLVM 
> doesn't serve them right now - I think it's unfortunate that this need for 
> the ABI hasn't translated into effort to finalise the ABI definition in the 
> psABI doc and to at least get it to match what GCC actually implements (spec. 
> That said, I've not really vocalised that concern clearly up to now - so 
> that's my bad.
>
> Matching what GCC does by setting stack alignment to 4 bytes for 2xlen types 
> seems fine - except this doesn't seem to be documented explicitly in the 
> current ABI doc (it notes the stack if 4 byte aligned, but you could have 
> that be the case and still require it to be realigned when storing objects 
> with a greater alignment requirement, surely?).
>
> Having different alignment requirements _only_ on the stack does seem ugly, 
> but I can't think of something off hand that would realistically break with 
> this.
>
> @wangpc do you want to update this with the suggested documentation in the 
> release notes and RISCVUsage on the support being "experimental"?

Thanks! I added a note to the RISCVUsage. There won't be `experimental-e` like 
other experimental extensions as it is already ratified and adds no 
instruction, it is experimental just because the support is experimental.

For ABI part, I don't know if @kito-cheng has some updates/comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70401

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

[PATCH] D70401: [RISCV] CodeGen of RVE and ilp32e/lp64e ABIs

[Alex Bradbury via Phabricator via cfe-commits]

asb added a comment.

First of all, thank you to everyone who has been trying to nudge this forward 
and apologies it must have been a frustrating experience.

I appreciate there are users who want to see this and I don't like that LLVM 
doesn't serve them right now - I think it's unfortunate that this need for the 
ABI hasn't translated into effort to finalise the ABI definition in the psABI 
doc and to at least get it to match what GCC actually implements (spec. That 
said, I've not really vocalised that concern clearly up to now - so that's my 
bad.

Matching what GCC does by setting stack alignment to 4 bytes for 2xlen types 
seems fine - except this doesn't seem to be documented explicitly in the 
current ABI doc (it notes the stack if 4 byte aligned, but you could have that 
be the case and still require it to be realigned when storing objects with a 
greater alignment requirement, surely?).

Having different alignment requirements _only_ on the stack does seem ugly, but 
I can't think of something off hand that would realistically break with this.

@wangpc do you want to update this with the suggested documentation in the 
release notes and RISCVUsage on the support being "experimental"?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70401

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

[PATCH] D70401: [RISCV] CodeGen of RVE and ilp32e/lp64e ABIs

[Craig Topper via Phabricator via cfe-commits]

craig.topper added inline comments.



Comment at: llvm/lib/Target/RISCV/RISCVISelLowering.cpp:16200
   SDValue Hi;
-  if (VA.getLocReg() == RISCV::X17) {
 // Second half of f64 is passed on the stack.

This code has been rewritten recently. Please rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70401

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

[PATCH] D70401: [RISCV] CodeGen of RVE and ilp32e/lp64e ABIs

[Wang Pengcheng via Phabricator via cfe-commits]

wangpc added a comment.

@asb @kito-cheng @jrtc27 @craig.topper 
Can I commit this since the support of RVE is really of great importance for 
some downstreams? If there are some problems, I will be there to fix them.
If we all agree with this, I will mark RVE as exprimental and commit it then.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70401

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

[PATCH] D70401: [RISCV] CodeGen of RVE and ilp32e/lp64e ABIs

[Zixuan Wu via Phabricator via cfe-commits]

zixuan-wu accepted this revision.
zixuan-wu added a comment.
This revision is now accepted and ready to land.

LGTM if nobody objects.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70401

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

[PATCH] D70401: [RISCV] CodeGen of RVE and ilp32e/lp64e ABIs

[Roman via Phabricator via cfe-commits]

kekcheburec added a comment.

In D70401#4653409 , @zixuan-wu wrote:

> ping?

Pong 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70401

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

[PATCH] D70401: [RISCV] CodeGen of RVE and ilp32e/lp64e ABIs

[Zixuan Wu via Phabricator via cfe-commits]

zixuan-wu added a comment.

ping?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70401

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

[PATCH] D70401: [RISCV] CodeGen of RVE and ilp32e/lp64e ABIs

[Luo Jia via Phabricator via cfe-commits]

luojia added a comment.

In D70401#4635875 , @koute wrote:

> I know that there are still open issues regarding the psABI, but considering 
> how slow it's been going, couldn't we merge this in anyway and mark it as 
> experimental and subject to change? Please?
>
> The patch is simple enough to not become a maintenance burden, and GCC 
> already has it even though the ABI's unfinished, and the RV32E target itself 
> is most likely going to be used for standalone bare metal programs where the 
> exact ABI shouldn't matter too much as long as it works.
>
> I'm asking because I'd **really** like to have this merged so that I could 
> use Rust to target RV32E/RV64E. Right now I have to maintain my own 
> toolchain, which is painful; if this got merged (even in an experimental 
> fashion, like GCC has) I could just get upstream Rust to support it 
> out-of-box.

I agree. Lots of our Rust work on low-level RISC-V cores (embedded, monitor 
hart, etc.) rely on RVE and they depend on RVE support on LLVM. We've waited 
for LLVM upstream support for an amount of years; considering how much time the 
community have waited for, RVE codegen can be accepted even if it's marked 
experimental.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70401

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

[PATCH] D70401: [RISCV] CodeGen of RVE and ilp32e/lp64e ABIs

[Craig Topper via Phabricator via cfe-commits]

craig.topper added inline comments.



Comment at: clang/lib/CodeGen/Targets/RISCV.cpp:479
+  // 2×XLEN-bit alignment and size at most 2×XLEN bits like `long long`,
+  // `unsigned long long` and `double` to have 4-bytes alignment. This
+  // behavior may be changed when RV32E/ILP32E is ratified.

4-bytes -> 4-byte



Comment at: clang/test/Preprocessor/riscv-target-features.c:6
 
+// CHECK-NOT: __riscv_32e
 // CHECK-NOT: __riscv_div {{.*$}}

__riscv_64e too



Comment at: llvm/lib/Support/RISCVISAInfo.cpp:937
   // TODO: The 'q' extension requires rv64.
-  // TODO: It is illegal to specify 'e' extensions with 'f' and 'd'.
 

This needs to be rebased. These FIXMEs were removed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70401

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

[PATCH] D70401: [RISCV] CodeGen of RVE and ilp32e/lp64e ABIs

[Wang Pengcheng via Phabricator via cfe-commits]

wangpc added a comment.

In D70401#4635875 , @koute wrote:

> I know that there are still open issues regarding the psABI, but considering 
> how slow it's been going, couldn't we merge this in anyway and mark it as 
> experimental and subject to change? Please?
>
> The patch is simple enough to not become a maintenance burden, and GCC 
> already has it even though the ABI's unfinished, and the RV32E target itself 
> is most likely going to be used for standalone bare metal programs where the 
> exact ABI shouldn't matter too much as long as it works.
>
> I'm asking because I'd **really** like to have this merged so that I could 
> use Rust to target RV32E/RV64E. Right now I have to maintain my own 
> toolchain, which is painful; if this got merged (even in an experimental 
> fashion, like GCC has) I could just get upstream Rust to support it 
> out-of-box.

@asb @kito-cheng @jrtc27 What do you think about?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70401

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

[PATCH] D70401: [RISCV] CodeGen of RVE and ilp32e/lp64e ABIs

[Koute via Phabricator via cfe-commits]

koute added a comment.

I know that there are still open issues regarding the psABI, but considering 
how slow it's been going, couldn't we merge this in anyway and mark it as 
experimental and subject to change? Please?

The patch is simple enough to not become a maintenance burden, and GCC already 
has it even though the ABI's unfinished, and the RV32E target itself is most 
likely going to be used for standalone bare metal programs where the exact ABI 
shouldn't matter too much as long as it works.

I'm asking because I'd **really** like to have this merged so that I could use 
Rust to target RV32E/RV64E. Right now I have to maintain my own toolchain, 
which is painful; if this got merged (even in an experimental fashion, like GCC 
has) I could just get upstream Rust to support it out-of-box.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70401

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

[PATCH] D70401: [RISCV] CodeGen of RVE and ilp32e/lp64e ABIs

[Zixuan Wu via Phabricator via cfe-commits]

zixuan-wu added inline comments.



Comment at: llvm/lib/Target/RISCV/RISCVRegisterInfo.cpp:66
   return CSR_XLEN_F32_Interrupt_SaveList;
 return CSR_Interrupt_SaveList;
   }

Here also need adjust for rve.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70401

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

[PATCH] D70401: [RISCV] CodeGen of RVE and ilp32e/lp64e ABIs

[Wang Pengcheng via Phabricator via cfe-commits]

wangpc added inline comments.



Comment at: llvm/lib/Target/RISCV/RISCVFrameLowering.cpp:908
 
 static const MCPhysReg CSRegs[] = { RISCV::X1,  /* ra */
   RISCV::X5, RISCV::X6, RISCV::X7,  /* t0-t2 */

zixuan-wu wrote:
> Hi, @wangpc it's hidden bug that out of range registers are saved/restored in 
> prologue/epilogue
Thanks! We don't need to save X16-X31 for interrupt functions.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70401

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

[PATCH] D70401: [RISCV] CodeGen of RVE and ilp32e/lp64e ABIs

[Zixuan Wu via Phabricator via cfe-commits]

zixuan-wu added inline comments.



Comment at: llvm/lib/Target/RISCV/RISCVFrameLowering.cpp:908
 
 static const MCPhysReg CSRegs[] = { RISCV::X1,  /* ra */
   RISCV::X5, RISCV::X6, RISCV::X7,  /* t0-t2 */

Hi, @wangpc it's hidden bug that out of range registers are saved/restored in 
prologue/epilogue


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70401

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

[PATCH] D70401: [RISCV] CodeGen of RVE and ilp32e/lp64e ABIs

[Wang Pengcheng via Phabricator via cfe-commits]

pcwang-thead added a comment.

In D70401#4411560 , @david-sawatzke 
wrote:

> Thank you for the reply, I've compiled this with the most recent patch and 
> also didn't see a problem (but can't get it running with rustc). Building the 
> .ll with the older patch, the same issue also occurs, so I *do* think its the 
> old patch version?
> Here is the log output for the riscv32e
> F27886013: llvm_output 
>
> (and as a sanity check riscv32i)
> F27886014: llvm_output_riscv32i 
>
> The errant code seems to get introduced here:
>
>   # *** IR Dump After Prologue/Epilogue Insertion & Frame Finalization 
> (prologepilog) ***:
>   # Machine code for function _ZN13miscomp_repro4test17h065760f827b95d43E: 
> NoPHIs, TracksLiveness, NoVRegs, TiedOpsRewritten, TracksDebugUserValues
>   
>   bb.0.start:
> $x2 = frame-destroy ADDI $x8, 0
> PseudoRET

Thanks! It seems that the problem is that we do wrong FP adjustment here.
But, as you can see, this patch does almost nothing to `RISCVFrameLowering`. So 
I think the bug may have been fixed somewhere else (I do remember there is a 
bug fix but I can't remember the differential ID).
So I would suggest you to use newest patch or do some bitsecting to find the 
bug fix commit if you don't bother. :-)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70401

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

[PATCH] D70401: [RISCV] CodeGen of RVE and ilp32e/lp64e ABIs

[David Sawatzke via Phabricator via cfe-commits]

david-sawatzke added a comment.

Thank you for the reply, I've compiled this with the most recent patch and also 
didn't see a problem (but can't get it running with rustc). Building the .ll 
with the older patch, the same issue also occurs, so I *do* think its the old 
patch version?
Here is the log output for the riscv32e
F27886013: llvm_output 

(and as a sanity check riscv32i)
F27886014: llvm_output_riscv32i 

The errant code seems to get introduced here:

  # *** IR Dump After Prologue/Epilogue Insertion & Frame Finalization 
(prologepilog) ***:
  # Machine code for function _ZN13miscomp_repro4test17h065760f827b95d43E: 
NoPHIs, TracksLiveness, NoVRegs, TiedOpsRewritten, TracksDebugUserValues
  
  bb.0.start:
$x2 = frame-destroy ADDI $x8, 0
PseudoRET


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70401

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

[PATCH] D70401: [RISCV] CodeGen of RVE and ilp32e/lp64e ABIs

[Wang Pengcheng via Phabricator via cfe-commits]

pcwang-thead added a comment.

In D70401#4409782 , @david-sawatzke 
wrote:

> Hey I've tried using this patch (roughly following 
> https://noxim.xyz/blog/rust-ch32v003/).
>
> It uses the older version of this patch for the rust llvm version (here the 
> llvm tree https://github.com/Noxime/llvm-project/tree/rv32e) and I use rust 
> commit 0939ec13 (together with the small patch for the RVE).
>
> I've experience some issues that results in corruption of $sp, the following 
> is the smallest reproduction (hopefully small enough):
> Code:
>
>   rust
>   #![no_std]
>   
>   pub fn test()  {
>   }
>
> which, with the following .ll for release builds:
>
>   source_filename = "miscomp_repro.8b6a426d3b54bd13-cgu.0"
>   target datalayout = "e-m:e-p:32:32-i64:64-n32-S128"
>   target triple = "riscv32"
>   
>   define dso_local void @_ZN13miscomp_repro4test17h065760f827b95d43E() 
> unnamed_addr #0 {
>   start:
> ret void
>   }
>   
>   attributes #0 = { mustprogress nofree norecurse nosync nounwind readnone 
> willreturn "target-cpu"="generic-rv32" "target-features"="+e,+c" }
>
> results in this assembly:
>
>   .text
>   .attribute  4, 4
>   .attribute  5, "rv32e1p9_c2p0"
>   .file   "miscomp_repro.8b6a426d3b54bd13-cgu.0"
>   .section
> .text._ZN13miscomp_repro4test17h065760f827b95d43E,"ax",@progbits
>   .globl  _ZN13miscomp_repro4test17h065760f827b95d43E
>   .p2align1
>   .type   _ZN13miscomp_repro4test17h065760f827b95d43E,@function
>   _ZN13miscomp_repro4test17h065760f827b95d43E:
>   mv  sp, s0
>   ret
>   .Lfunc_end0:
>   .size   _ZN13miscomp_repro4test17h065760f827b95d43E, 
> .Lfunc_end0-_ZN13miscomp_repro4test17h065760f827b95d43E
>   
>   .section".note.GNU-stack","",@progbits
>
> Since s0 isn't required to have any specific contents (and in the larger 
> project this was extracted from doesn't), this corrupts the stack pointer. 
> Large functions using the stack first save sp to  0, so not all functions 
> have this issue. This also happens (but more verbose) in debug builds, but 
> works fine with the exact same toolchain using the riscv32i target.
>
> Here is the repro with some further output, I hope this patch and not 
> something else is to blame (if so, sorry in advance).
>
> F27877626: miscomp_repro.zip 

Thanks for reporting this.
I tried to compile your .ll on my local machine with newest patch, I didn't see 
the problem. I don't know if it is the bug in older version of this patch, so I 
suggest you to update the patch and try again. :-)
By the way, you can provide the log when you compile the .ll with `-mllvm 
-print-after-all` option (and `-mllvm -debug` if your llvm is a debug build). 
It can be helpful for me to figure out which part is wrong.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70401

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

[PATCH] D70401: [RISCV] CodeGen of RVE and ilp32e/lp64e ABIs

[David Sawatzke via Phabricator via cfe-commits]

david-sawatzke added a comment.

Hey I've tried using this patch (roughly following 
https://noxim.xyz/blog/rust-ch32v003/).

It uses the older version of this patch for the rust llvm version (here the 
llvm tree https://github.com/Noxime/llvm-project/tree/rv32e) and I use rust 
commit 0939ec13 (together with the small patch for the RVE).

I've experience some issues that results in corruption of $sp, the following is 
the smallest reproduction (hopefully small enough):
Code:

  rust
  #![no_std]
  
  pub fn test()  {
  }

which, with the following .ll for release builds:

  source_filename = "miscomp_repro.8b6a426d3b54bd13-cgu.0"
  target datalayout = "e-m:e-p:32:32-i64:64-n32-S128"
  target triple = "riscv32"
  
  define dso_local void @_ZN13miscomp_repro4test17h065760f827b95d43E() 
unnamed_addr #0 {
  start:
ret void
  }
  
  attributes #0 = { mustprogress nofree norecurse nosync nounwind readnone 
willreturn "target-cpu"="generic-rv32" "target-features"="+e,+c" }

results in this assembly:

.text
.attribute  4, 4
.attribute  5, "rv32e1p9_c2p0"
.file   "miscomp_repro.8b6a426d3b54bd13-cgu.0"
.section
.text._ZN13miscomp_repro4test17h065760f827b95d43E,"ax",@progbits
.globl  _ZN13miscomp_repro4test17h065760f827b95d43E
.p2align1
.type   _ZN13miscomp_repro4test17h065760f827b95d43E,@function
  _ZN13miscomp_repro4test17h065760f827b95d43E:
mv  sp, s0
ret
  .Lfunc_end0:
.size   _ZN13miscomp_repro4test17h065760f827b95d43E, 
.Lfunc_end0-_ZN13miscomp_repro4test17h065760f827b95d43E
  
.section".note.GNU-stack","",@progbits

Since s0 isn't required to have any specific contents (and in the larger 
project this was extracted from doesn't), this corrupts the stack pointer. 
Large functions using the stack first save sp to  0, so not all functions have 
this issue. This also happens (but more verbose) in debug builds, but works 
fine with the exact same toolchain using the riscv32i target.

Here is the repro with some further output, I hope this patch and not something 
else is to blame (if so, sorry in advance).

F27877626: miscomp_repro.zip 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70401

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

[PATCH] D70401: [RISCV] CodeGen of RVE and ilp32e/lp64e ABIs

[Roman via Phabricator via cfe-commits]

kekcheburec added a comment.

In D70401#4226597 , @pcwang-thead 
wrote:

> In D70401#4226549 , @recallmenot 
> wrote:
>
>> In D70401#4205333 , @pcwang-thead 
>> wrote:
>>
>>> In D70401#4204511 , @recallmenot 
>>> wrote:
>>>
 Hi, I'm working on CH32V003 for rust and it uses RV32EC core.
 I tried replacing my distros llvm and clang with a patched version of this 
 like this:

   git clone https://aur.archlinux.org/llvm-git.git
   cd llvm-git
   mkdir src
   cd src
   git clone https://github.com/llvm/llvm-project.git
   cd llvm-project
   arc patch D70401
   cd ../..
   mv llvm-config.h src/
   makepkg -es
   sudo pacman -Rd --nodeps clang llvm
   makepkg -eid

 but that bricked my xfce-wayland-manjaro DE (one screen black)
 And in config.toml if I put

   [build]
   target = "riscv32i-unknown-none-elf"
   rustflags = [
"-C", "target-feature=+e,+c"
   ]

 then build with cargo build
 LLVM still complains it doesn't implement CodeGen for RV32E yet
 What am I doing wrong?
 Ended up reverting to repository llvm and clang, desktop now works again 
 but CodeGen is obviously missing.
>>>
>>> I don't see any obvious problem here.
>>> I am not familiar with rust. Is `riscv32i-unknown-none-elf` a valid target 
>>> for `rustc`, it should be something like `riscv32-unknown-elf` in LLVM I 
>>> think. And is `target-feature=+e,+c` the right way to specify features?
>>> Can you please provide the whole command/arguments passed to LLVM?
>>
>> Yeah so I looked at the at the target files of rustc, telling rustc to do 
>> RV32I will indeed result in RV32 and the way to enable the E and C features 
>> seems to be correct, BUT:
>> rust uses their own "special sauce" version of llvm and rustc needs to be 
>> built against that to enable the new features. I tried to apply (patch) the 
>> diff directly to rusts llvm branch but there were many errors, and I 
>> couldn't figure out how to apply them manually since some things are 
>> different.
>> I'm stuck, this is all way beyond my understanding. Sorry I can't test it 
>> for you guys.
>> What I did was:
>>
>>   git clone https://github.com/rust-lang/rust.git
>>   cd rust
>>   nvim config.toml
>>
>>   [llvm]
>>   download-ci-llvm = false
>>
>> then I started building with
>>
>>   ./x.py build
>>
>> and as soon as the rust-llvm source was downloaded completely I aborted 
>> (CTRL+C).
>>
>> then downloaded the raw diff from this page (button top right) into the rust 
>> llvm dir, opened a terminal in that dir and tried to patch with
>>
>>   patch -p1 < D70401.diff
>>
>> but that gives lots of errors
>> resolving them manually seems way beyond me, especially since patch seems to 
>> already use fuzzy matching
>
> So it seems that rust uses its own llvm branch based on released llvm branch, 
> so I think you may download old version of this patch which is near the 
> baseline of rust llvm branch and try again. :-)

I made a fork with a fix (until the recent rebase with the addition of rv64e) 
https://github.com/kekcheburec/llvm-project/tree/5cf27e03900c06c0f374a8f3a6e65a817ce70607

In this case, you need to use a fork https://github.com/rust-lang/llvm-project 
because this is the latest version of LLVM that has not yet been switched to. 
For older versions of LLVM, you can also use pure LLVM.

Do you have any news about adding rv32e/rv64e to LLD? At the moment, you need 
to use GCC for linking.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70401

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

[PATCH] D70401: [RISCV] CodeGen of RVE and ilp32e/lp64e ABIs

[Wang Pengcheng via Phabricator via cfe-commits]

pcwang-thead added a comment.

In D70401#4226549 , @recallmenot wrote:

> In D70401#4205333 , @pcwang-thead 
> wrote:
>
>> In D70401#4204511 , @recallmenot 
>> wrote:
>>
>>> Hi, I'm working on CH32V003 for rust and it uses RV32EC core.
>>> I tried replacing my distros llvm and clang with a patched version of this 
>>> like this:
>>>
>>>   git clone https://aur.archlinux.org/llvm-git.git
>>>   cd llvm-git
>>>   mkdir src
>>>   cd src
>>>   git clone https://github.com/llvm/llvm-project.git
>>>   cd llvm-project
>>>   arc patch D70401
>>>   cd ../..
>>>   mv llvm-config.h src/
>>>   makepkg -es
>>>   sudo pacman -Rd --nodeps clang llvm
>>>   makepkg -eid
>>>
>>> but that bricked my xfce-wayland-manjaro DE (one screen black)
>>> And in config.toml if I put
>>>
>>>   [build]
>>>   target = "riscv32i-unknown-none-elf"
>>>   rustflags = [
>>> "-C", "target-feature=+e,+c"
>>>   ]
>>>
>>> then build with cargo build
>>> LLVM still complains it doesn't implement CodeGen for RV32E yet
>>> What am I doing wrong?
>>> Ended up reverting to repository llvm and clang, desktop now works again 
>>> but CodeGen is obviously missing.
>>
>> I don't see any obvious problem here.
>> I am not familiar with rust. Is `riscv32i-unknown-none-elf` a valid target 
>> for `rustc`, it should be something like `riscv32-unknown-elf` in LLVM I 
>> think. And is `target-feature=+e,+c` the right way to specify features?
>> Can you please provide the whole command/arguments passed to LLVM?
>
> Yeah so I looked at the at the target files of rustc, telling rustc to do 
> RV32I will indeed result in RV32 and the way to enable the E and C features 
> seems to be correct, BUT:
> rust uses their own "special sauce" version of llvm and rustc needs to be 
> built against that to enable the new features. I tried to apply (patch) the 
> diff directly to rusts llvm branch but there were many errors, and I couldn't 
> figure out how to apply them manually since some things are different.
> I'm stuck, this is all way beyond my understanding. Sorry I can't test it for 
> you guys.
> What I did was:
>
>   git clone https://github.com/rust-lang/rust.git
>   cd rust
>   nvim config.toml
>
>   [llvm]
>   download-ci-llvm = false
>
> then I started building with
>
>   ./x.py build
>
> and as soon as the rust-llvm source was downloaded completely I aborted 
> (CTRL+C).
>
> then downloaded the raw diff from this page (button top right) into the rust 
> llvm dir, opened a terminal in that dir and tried to patch with
>
>   patch -p1 < D70401.diff
>
> but that gives lots of errors
> resolving them manually seems way beyond me, especially since patch seems to 
> already use fuzzy matching

So it seems that rust uses its own llvm branch based on released llvm branch, 
so I think you may download old version of this patch which is near the 
baseline of rust llvm branch and try again. :-)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70401

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

[PATCH] D70401: [RISCV] CodeGen of RVE and ilp32e/lp64e ABIs

[Edmund Raile via Phabricator via cfe-commits]

recallmenot added a comment.

In D70401#4205333 , @pcwang-thead 
wrote:

> In D70401#4204511 , @recallmenot 
> wrote:
>
>> Hi, I'm working on CH32V003 for rust and it uses RV32EC core.
>> I tried replacing my distros llvm and clang with a patched version of this 
>> like this:
>>
>>   git clone https://aur.archlinux.org/llvm-git.git
>>   cd llvm-git
>>   mkdir src
>>   cd src
>>   git clone https://github.com/llvm/llvm-project.git
>>   cd llvm-project
>>   arc patch D70401
>>   cd ../..
>>   mv llvm-config.h src/
>>   makepkg -es
>>   sudo pacman -Rd --nodeps clang llvm
>>   makepkg -eid
>>
>> but that bricked my xfce-wayland-manjaro DE (one screen black)
>> And in config.toml if I put
>>
>>   [build]
>>   target = "riscv32i-unknown-none-elf"
>>   rustflags = [
>>  "-C", "target-feature=+e,+c"
>>   ]
>>
>> then build with cargo build
>> LLVM still complains it doesn't implement CodeGen for RV32E yet
>> What am I doing wrong?
>> Ended up reverting to repository llvm and clang, desktop now works again but 
>> CodeGen is obviously missing.
>
> I don't see any obvious problem here.
> I am not familiar with rust. Is `riscv32i-unknown-none-elf` a valid target 
> for `rustc`, it should be something like `riscv32-unknown-elf` in LLVM I 
> think. And is `target-feature=+e,+c` the right way to specify features?
> Can you please provide the whole command/arguments passed to LLVM?

Yeah so I looked at the at the target files of rustc, telling rustc to do RV32I 
will indeed result in RV32 and the way to enable the E and C features seems to 
be correct, BUT:
rust uses their own "special sauce" version of llvm and rustc needs to be built 
against that to enable the new features. I tried to apply (patch) the diff 
directly to rusts llvm branch but there were many errors, and I couldn't figure 
out how to apply them manually since some things are different.
I'm stuck, this is all way beyond my understanding. Sorry I can't test it for 
you guys.
What I did was:

  git clone https://github.com/rust-lang/rust.git
  cd rust
  nvim config.toml

  [llvm]
  download-ci-llvm = false

then I started building with

  ./x.py build

and as soon as the rust-llvm source was downloaded completely I aborted 
(CTRL+C).

then downloaded the raw diff from this page (button top right) into the rust 
llvm dir, opened a terminal in that dir and tried to patch with

  patch -p1 < D70401.diff

but that gives lots of errors
resolving them manually seems way beyond me, especially since patch seems to 
already use fuzzy matching


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70401

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