Page MenuHomePhabricator

luismarques (Luís Marques)
Software/hardware engineer at lowRISC

Projects

User does not belong to any projects.

User Details

User Since
Nov 2 2018, 7:48 AM (102 w, 6 d)

Mostly LLVM work.

Recent Activity

Today

luismarques committed rG89840380d56e: [RISCV][NFC] Add more tests for 32-bit constant materialization (authored by luismarques).
[RISCV][NFC] Add more tests for 32-bit constant materialization
Thu, Oct 22, 3:37 AM
luismarques closed D83210: [RISCV][NFC] Add more tests for 32-bit constant materialization.
Thu, Oct 22, 3:36 AM · Restricted Project

Yesterday

luismarques committed rG58f6b16c4981: [compiler-rt][builtins][RISCV] Always include __mul[sd]i3 builtin definitions (authored by luismarques).
[compiler-rt][builtins][RISCV] Always include __mul[sd]i3 builtin definitions
Wed, Oct 21, 1:50 AM
luismarques closed D86457: [compiler-rt][builtins][RISCV] Always include __mul[sd]i3 builtin definitions.
Wed, Oct 21, 1:49 AM · Restricted Project

Tue, Oct 20

luismarques committed rGfc3f9dfad33d: [compiler-rt][builtins] Add tests for atomic builtins support functions (authored by luismarques).
[compiler-rt][builtins] Add tests for atomic builtins support functions
Tue, Oct 20, 4:10 AM
luismarques closed D86278: [compiler-rt][builtins] Add tests for atomic builtins support functions.
Tue, Oct 20, 4:09 AM · Restricted Project

Mon, Oct 19

luismarques committed rG7ddd354d47cc: [RISCV][ASAN] Fix TLS offsets (authored by luismarques).
[RISCV][ASAN] Fix TLS offsets
Mon, Oct 19, 5:42 AM
luismarques closed D89244: [RISCV][ASAN] Fix TLS offsets.
Mon, Oct 19, 5:42 AM · Restricted Project

Sat, Oct 17

luismarques committed rGb7ff218f1c04: [RISCV][ASAN] Fix passing XFAIL tests (authored by luismarques).
[RISCV][ASAN] Fix passing XFAIL tests
Sat, Oct 17, 8:55 AM
luismarques closed D89299: [RISCV][ASAN] Fix passing XFAIL tests.
Sat, Oct 17, 8:55 AM · Restricted Project

Wed, Oct 14

luismarques accepted D89025: [RISCV] Add -mtune support.
Wed, Oct 14, 1:58 AM · Restricted Project, Restricted Project

Tue, Oct 13

luismarques added a comment to D89025: [RISCV] Add -mtune support.

RISCV supports -mcpu with default empty arch to align gcc's -mtune behavior since clang didn't support -mtune before. But now clang has -mtune, is it a good idea to remove those options? (ex. rocket-rv32/rv64, sifive-7-rv32/64)

Tue, Oct 13, 8:27 AM · Restricted Project, Restricted Project
luismarques accepted D89025: [RISCV] Add -mtune support.

LGTM, but I would like other people to also review this, if possible.
(Just be sure to check/fix the clang-format warnings and the inline comments).

Tue, Oct 13, 8:11 AM · Restricted Project, Restricted Project
luismarques added a comment to D89244: [RISCV][ASAN] Fix TLS offsets.

Thanks for the info @kito-cheng

Tue, Oct 13, 1:58 AM · Restricted Project
luismarques updated the diff for D89244: [RISCV][ASAN] Fix TLS offsets.

Use __builtin_thread_pointer.

Tue, Oct 13, 1:57 AM · Restricted Project
luismarques requested review of D89299: [RISCV][ASAN] Fix passing XFAIL tests.
Tue, Oct 13, 12:58 AM · Restricted Project

Mon, Oct 12

luismarques added a comment to D89239: [RISCV][PrologEpilogInserter] "Float" emergency spill slots to avoid making them immediately unreachable from the stack pointer.

The code makes sense to me and it seems correct, but there may be assumptions about the way that mechanism operates that I'm not familiar with. Some comments:

Mon, Oct 12, 8:51 AM · Restricted Project
luismarques accepted D89237: [RISCV] Do not grow the stack a second time when we need to realign the stack.

LGTM. Thanks for the detailed description, with the links.

Mon, Oct 12, 8:25 AM · Restricted Project
luismarques added inline comments to D89244: [RISCV][ASAN] Fix TLS offsets.
Mon, Oct 12, 8:12 AM · Restricted Project
luismarques updated the diff for D89244: [RISCV][ASAN] Fix TLS offsets.

NFC

Mon, Oct 12, 8:10 AM · Restricted Project
luismarques requested review of D89244: [RISCV][ASAN] Fix TLS offsets.
Mon, Oct 12, 8:07 AM · Restricted Project
luismarques added a comment to D86198: llvm enable sanitizer (RISCV64).

RISC-V support for ASan has been merged (see D87582 and its predecessors). This patch should probably be abandoned. Thank you.

Mon, Oct 12, 7:52 AM
luismarques added a comment to D86195: [RISC-V] Add support for AddressSanitizer on RISC-V GCC.

RISC-V support for ASan has been merged (see D87582 and its predecessors). This patch should probably be abandoned. Thank you.

Mon, Oct 12, 7:51 AM

Fri, Oct 9

luismarques accepted D89090: [RISCV] Only return DestSourcePair from isCopyInstrImpl for registers.

Thanks for the fix. Sorry for not catching that during the D86522 review.

Fri, Oct 9, 4:04 AM · Restricted Project
luismarques added a comment to D86278: [compiler-rt][builtins] Add tests for atomic builtins support functions.

Under what circumstances does this test actually get built? By default, the builtins library doesn't provide these functions, so this will fail to link.

Fri, Oct 9, 2:17 AM · Restricted Project
luismarques committed rG32f2f0d78aa0: [NFC] Fix banner (authored by luismarques).
[NFC] Fix banner
Fri, Oct 9, 2:00 AM

Thu, Oct 8

luismarques added a comment to D83210: [RISCV][NFC] Add more tests for 32-bit constant materialization.

Ping

Thu, Oct 8, 9:02 AM · Restricted Project
luismarques added a comment to D86457: [compiler-rt][builtins][RISCV] Always include __mul[sd]i3 builtin definitions.

Ping.

Thu, Oct 8, 6:33 AM · Restricted Project
luismarques added a comment to D86278: [compiler-rt][builtins] Add tests for atomic builtins support functions.

Ping

Thu, Oct 8, 2:44 AM · Restricted Project

Wed, Oct 7

luismarques accepted D87997: [RISCV][crt] support building without init_array.

LGTM. Just remove nop, ensure it isn't actually needed (rerun the tests, etc.), and feel free to commit this patch. Thanks!

Wed, Oct 7, 6:23 AM · Restricted Project

Thu, Oct 1

luismarques accepted D87580: [RISCV][ASAN] support code for architecture-specific parts of asan.

For versions 2.29 -- 2.31 you have val = 1172. Is that actually correct? I thought you had earlier obtained a value of 1772 for 2.29, so maybe that's a typo?
Other than that issue, LGTM. Just be sure to verify/adjust that.

Thu, Oct 1, 3:44 PM · Restricted Project

Tue, Sep 29

luismarques added a comment to D87577: [RISCV][ASAN] implementation for previous/next pc routines for riscv64.

By the way - to my surprise, Chris gave me write permissions - So I can commit too.

Tue, Sep 29, 10:38 AM · Restricted Project
luismarques accepted D87577: [RISCV][ASAN] implementation for previous/next pc routines for riscv64.

LGTM. I'll commit this.

Tue, Sep 29, 9:50 AM · Restricted Project

Thu, Sep 24

luismarques committed rG303e8cdacb10: [NFC][RISCV][builtins] Remove some hard-coded values from i-cache clear routine (authored by smd).
[NFC][RISCV][builtins] Remove some hard-coded values from i-cache clear routine
Thu, Sep 24, 6:32 AM
luismarques closed D87578: [NFC][RISCV][builtins] remove some hard-coded values from i-cache clear routine.
Thu, Sep 24, 6:32 AM · Restricted Project
luismarques added inline comments to D87580: [RISCV][ASAN] support code for architecture-specific parts of asan.
Thu, Sep 24, 4:31 AM · Restricted Project
luismarques accepted D87577: [RISCV][ASAN] implementation for previous/next pc routines for riscv64.

I haven't had the opportunity to personally test this, but looking at the code and the information you have provided I think it's probably fine. Please just adjust the code comment.
LGTM.

Thu, Sep 24, 2:31 AM · Restricted Project
luismarques added a comment to D87578: [NFC][RISCV][builtins] remove some hard-coded values from i-cache clear routine.

I can commit this for you. Is assume this the correct author? Alexey Baturo <space.monkey.delivers@gmail.com>

Thu, Sep 24, 2:16 AM · Restricted Project
luismarques accepted D87578: [NFC][RISCV][builtins] remove some hard-coded values from i-cache clear routine.

LGTM. Thanks for the patch.

Thu, Sep 24, 2:01 AM · Restricted Project

Wed, Sep 23

luismarques added inline comments to D87580: [RISCV][ASAN] support code for architecture-specific parts of asan.
Wed, Sep 23, 6:26 AM · Restricted Project

Sep 22 2020

luismarques added a comment to D62732: [RISCV] Add SystemV ABI.

Sounds reasonable to me. Maybe it's still to early for that but have you tried running (part of) the test suite under QEMU yet? It should give you a pretty good idea of the state of things and gives you a bunch of test coverage for free. You'll probably want to consider setting up a bot for that down the road anyway.

Sep 22 2020, 9:10 AM · Restricted Project
luismarques added a comment to D62732: [RISCV] Add SystemV ABI.

@luismarques , what's the recommended gdb-server implementation you recommend for me to try this on a riscv machine?

Sep 22 2020, 4:01 AM · Restricted Project

Sep 18 2020

luismarques added a comment to D80465: [RISCV-V] Provide muldi3 builtins for riscv.

Do other architectures with native 64-bit multiplication provide __muldi3? If yes, it should be re-enabled; if no, the test should be fixed. My assumption is it's the latter, but I can see the argument for having an RVM compiler-rt still provide __muldi3 in case linked against code that was compiled without RVM (but, really, that code should just be compiled with RVM).

Sep 18 2020, 4:09 PM · Restricted Project
luismarques added a comment to D80465: [RISCV-V] Provide muldi3 builtins for riscv.

I noticed that this patch (along with D86036) caused a compiler-rt test to fail for riscv64 when M extension is enabled.
compiler-rt/test/builtins/Unit/muldi3_test.c:11: undefined reference to `muldi3'
Basically, with this patch,
muldi3 is no longer defined anymore for riscv64 when M extension is enabled. Any suggestion on how this should be fixed?

Sep 18 2020, 4:03 PM · Restricted Project

Sep 17 2020

luismarques added a comment to D85095: Fix libcxx build on riscv32.

I suppose that an alternative to this implementation might be to tweak the place of use of __NR_futex, like is done in [1]. Arguably that's cleaner. Or, in the place where you're currently checking for riscv32, check for the case where __NR_futex_ isn't defined but __NR_futex_time64 is, and #define the former in terms of the latter.

Sep 17 2020, 8:44 AM · Restricted Project
luismarques added a comment to D85095: Fix libcxx build on riscv32.

I think this makes sense, but I'll let the #libc reviewers actually review/approve this.

Sep 17 2020, 2:40 AM · Restricted Project
luismarques added a comment to D80690: [RISCV] Support libunwind for riscv32.

This seems roughly OK now. Does anyone with libunwind experience want to give it a thumbs up?
#libunwind people, does this LGTY?

Sep 17 2020, 2:08 AM · Restricted Project, Restricted Project
luismarques accepted D86522: [RISC-V] Implement RISCVInstrInfo::isCopyInstrImpl().

I was hoping others would chime in regarding what instructions should be handled in this hook.

Sep 17 2020, 1:55 AM · Restricted Project

Sep 14 2020

luismarques added a comment to D87576: [RISCV][ASAN] implementation of SignalContext::GetWriteFlag.

There is already an implementation of GetWriteFlag for RISC-V. It was provided by D75168, and you can find it in lines 2002-2105.

Sep 14 2020, 7:12 AM · Restricted Project
luismarques added inline comments to D87577: [RISCV][ASAN] implementation for previous/next pc routines for riscv64.
Sep 14 2020, 6:56 AM · Restricted Project
luismarques added inline comments to D87579: [RISCV][ASAN] unwind fixup.
Sep 14 2020, 6:43 AM · Restricted Project
luismarques requested changes to D87580: [RISCV][ASAN] support code for architecture-specific parts of asan.
Sep 14 2020, 6:15 AM · Restricted Project
luismarques added inline comments to D87582: [RISCV][ASAN] mark asan as supported for RISCV64 and enable tests.
Sep 14 2020, 5:07 AM · Restricted Project
luismarques added a comment to D87578: [NFC][RISCV][builtins] remove some hard-coded values from i-cache clear routine.

You'll notice that there is already an implementation of __clear_cache for RISC-V in the file you are contributing to, in line 160. That existing implementation directly performs a syscall. Your new implementation calls a (RISC-V-specific) GNU libc function __riscv_flush_icache, which internally either uses a vDSO (if available) or performs the syscall.

Sep 14 2020, 3:16 AM · Restricted Project

Sep 8 2020

luismarques updated the diff for D86278: [compiler-rt][builtins] Add tests for atomic builtins support functions.
  • Add missing return from main.
Sep 8 2020, 7:27 AM · Restricted Project
luismarques added a comment to D86278: [compiler-rt][builtins] Add tests for atomic builtins support functions.

Ping. I think all of the review issues (originally added in D81348) have been addressed so this is probably ready to land.

Sep 8 2020, 7:18 AM · Restricted Project
luismarques updated the diff for D86457: [compiler-rt][builtins][RISCV] Always include __mul[sd]i3 builtin definitions.

Address review concerns.

Sep 8 2020, 6:40 AM · Restricted Project
luismarques added inline comments to D86457: [compiler-rt][builtins][RISCV] Always include __mul[sd]i3 builtin definitions.
Sep 8 2020, 6:39 AM · Restricted Project
luismarques added a comment to D86198: llvm enable sanitizer (RISCV64).

There are still issues with compiling and running the sanitizer tests for the added architecture. I think fixing those should be a precondition for this to be considered mergeable.

Have you added the flag "-DLLVM_DEFAULT_TARGET_TRIPLE="riscv64-unknown-linux-gnu"" and "-fsanitize=address" when compiling the sanitizer test?

Sep 8 2020, 4:17 AM

Sep 3 2020

luismarques added a comment to D62732: [RISCV] Add SystemV ABI.

@labath @jrtc27 @clayborg Now that we have at least 3 open-source debug servers that we can use to test this with (OpenOCD, QEMU gdbstub, gdbserver) perhaps this can be merged? I had very good results using this patch with OpenOCD. This patch doesn't include automated tests, but I'm not sure what tests would be required for this patch, or that it makes sense to require them at this point. I'll be doing more work for LLDB RISC-V support, and I'll provide tests for specific fixes going forward.

Sep 3 2020, 6:01 AM · Restricted Project
luismarques updated the diff for D62732: [RISCV] Add SystemV ABI.
  • Fix list of callee saved registers;
  • Fix typo;
  • Fix trivial clang-format issue.
Sep 3 2020, 5:49 AM · Restricted Project
luismarques commandeered D62732: [RISCV] Add SystemV ABI.

Commandeering the patch, as discussed in the last RISC-V sync-up call. I'll update it with only minor changes; further development can be done in follow-up patches.

Sep 3 2020, 5:42 AM · Restricted Project

Sep 2 2020

luismarques added inline comments to D86522: [RISC-V] Implement RISCVInstrInfo::isCopyInstrImpl().
Sep 2 2020, 6:44 AM · Restricted Project

Aug 28 2020

luismarques added a reviewer for D86278: [compiler-rt][builtins] Add tests for atomic builtins support functions: efriedma.
Aug 28 2020, 8:38 AM · Restricted Project
luismarques added a comment to D86198: llvm enable sanitizer (RISCV64).

I have updated my patch and summary. Are there any new updates with the patch review?

Aug 28 2020, 7:47 AM
luismarques requested changes to D86195: [RISC-V] Add support for AddressSanitizer on RISC-V GCC.
Aug 28 2020, 7:23 AM

Aug 27 2020

luismarques resigned from D86510: [compiler-rt] Fix atomic support functions on 32-bit architectures.

I'll leave this to the experts then! :-)

Aug 27 2020, 6:31 AM · Restricted Project
luismarques added a reviewer for D86510: [compiler-rt] Fix atomic support functions on 32-bit architectures: jyknight.
Aug 27 2020, 6:25 AM · Restricted Project

Aug 25 2020

luismarques added a comment to D86292: [LLDB][RISCV] Distinguish between riscv32 and riscv64 based on ELF class.

Not so silly; gdb (well, the names are inherited from bfd) has set architecture riscv:rv32/set architecture riscv:rv64 :)

Aug 25 2020, 2:57 PM · Restricted Project
luismarques added a comment to D86522: [RISC-V] Implement RISCVInstrInfo::isCopyInstrImpl().

This is technically correct, but looking at the other targets it seems that the hook is generally being used more restrictively for instructions where you actually have some expectation of it being a copy/move. Do we really want to go down this route? In principle we'd have to later expand this to e.g. all of the bitmanip instructions that can implement copies and so on, which seems a bit like a reductio ad absurdum. Having tests showing at least a little bit of benefit would help make the case for this patch :-)

Aug 25 2020, 8:13 AM · Restricted Project
luismarques added inline comments to D86281: [compiler-rt][builtins] Tweak checks for lock-free atomics.
Aug 25 2020, 4:59 AM · Restricted Project
luismarques accepted D86517: [RISC-V] Mark C_MV as a move instruction.

LGTM.

Aug 25 2020, 4:43 AM · Restricted Project
luismarques accepted D86480: [RISC-V] ADDI/ORI/XORI x, 0 should be as cheap as a move.

Looking at the AArch64 implementation, it seems there is precedent for this hook already including other instructions that are cheap but not necessarily recognized as actual moves by the microarchitecture (i.e. renames). So I guess they may not be, strictly speaking, as cheap as a move, but they are close enough. And that may very well be the point of the hook, otherwise they would just be non-canonical moves?

Aug 25 2020, 2:28 AM · Restricted Project
luismarques accepted D86510: [compiler-rt] Fix atomic support functions on 32-bit architectures.

LGTM. (I had already gone down roughly the same path in D86281. This patch additionally adds checks for the alignments of the pointers, so it can supersede D86281)

Aug 25 2020, 1:37 AM · Restricted Project
luismarques abandoned D86281: [compiler-rt][builtins] Tweak checks for lock-free atomics.

Only just saw this after posting my own fix (D86510). This LGTM, and I can rebase my change to enable the 16-byte case on top of this.

Aug 25 2020, 1:31 AM · Restricted Project

Aug 24 2020

luismarques added a comment to D86036: [compiler-rt][RISCV] Use muldi3 builtin assembly implementation.

I meant muldi3.S and mulsi3.S, not the .inc file - I don't think either is changed by that patch yet?

Aug 24 2020, 7:25 AM · Restricted Project
luismarques added a comment to D86036: [compiler-rt][RISCV] Use muldi3 builtin assembly implementation.

@thakis this also matches the #ifdefs in the files themselves.

Aug 24 2020, 7:13 AM · Restricted Project
luismarques requested review of D86457: [compiler-rt][builtins][RISCV] Always include __mul[sd]i3 builtin definitions.
Aug 24 2020, 6:28 AM · Restricted Project
luismarques added inline comments to D86036: [compiler-rt][RISCV] Use muldi3 builtin assembly implementation.
Aug 24 2020, 5:59 AM · Restricted Project

Aug 21 2020

luismarques committed rG57903cf09335: [compiler-rt][RISCV] Use muldi3 builtin assembly implementation (authored by luismarques).
[compiler-rt][RISCV] Use muldi3 builtin assembly implementation
Aug 21 2020, 5:07 AM
luismarques closed D86036: [compiler-rt][RISCV] Use muldi3 builtin assembly implementation.
Aug 21 2020, 5:07 AM · Restricted Project
luismarques updated the diff for D86292: [LLDB][RISCV] Distinguish between riscv32 and riscv64 based on ELF class.

Add riscv32 test.

Aug 21 2020, 3:43 AM · Restricted Project

Aug 20 2020

luismarques requested review of D86292: [LLDB][RISCV] Distinguish between riscv32 and riscv64 based on ELF class.
Aug 20 2020, 7:57 AM · Restricted Project
luismarques abandoned D86043: [compiler-rt][builtins] Tweak checks for lock-free atomics.

This review is superseded by D86281, due to issues with subscribing llvm-commits.

Aug 20 2020, 5:25 AM · Restricted Project
luismarques requested review of D86281: [compiler-rt][builtins] Tweak checks for lock-free atomics.
Aug 20 2020, 5:24 AM · Restricted Project
luismarques abandoned D81348: [compiler-rt][builtins] Add tests for atomic builtins support functions.

This is superseded by D86278, due to issues with subscribing llvm-commits.

Aug 20 2020, 3:49 AM · Restricted Project
luismarques requested review of D86278: [compiler-rt][builtins] Add tests for atomic builtins support functions.
Aug 20 2020, 3:46 AM · Restricted Project
luismarques added a comment to D86198: llvm enable sanitizer (RISCV64).

For D84727, I didn't know how to correctly submit an updated patch before. That is why I submitted another patch. For the current stage, you can just abandon D84727.

Aug 20 2020, 3:02 AM
luismarques added inline comments to D81348: [compiler-rt][builtins] Add tests for atomic builtins support functions.
Aug 20 2020, 2:56 AM · Restricted Project
luismarques added a comment to D86198: llvm enable sanitizer (RISCV64).

Did you mean D84727? For that patch, no one has given an update feedback so far.

Aug 20 2020, 2:22 AM

Aug 19 2020

luismarques requested changes to D86198: llvm enable sanitizer (RISCV64).
  1. You need to provide the full context in the patch. See [1], particularly the part about the -U999999.
  2. You need to base your patch on a recent LLVM commit, as this patch doesn't apply cleanly.
  3. This patch repeats changes from D86198. Are you abandoning D86198? If so you need to update the status of that review.
  4. I would expect this review to provide a more detailed summary, including the support status of the various sanitizers for RISC-V, and why it's reasonable to enable them.
Aug 19 2020, 2:52 AM

Aug 18 2020

luismarques added a comment to D86043: [compiler-rt][builtins] Tweak checks for lock-free atomics.
In D86043#2222622, @jfb wrote:

Can you add a test that this works, e.g. by having a program which uses these functions, but doesn't link to libatomic?

Aug 18 2020, 2:16 AM · Restricted Project

Aug 17 2020

luismarques committed rG687e7d34253b: [NFC] Tweak a comment about the lock-free builtins (authored by luismarques).
[NFC] Tweak a comment about the lock-free builtins
Aug 17 2020, 5:45 AM
luismarques added a reviewer for D86043: [compiler-rt][builtins] Tweak checks for lock-free atomics: eli.friedman.
Aug 17 2020, 5:41 AM · Restricted Project

Aug 16 2020

luismarques retitled D86043: [compiler-rt][builtins] Tweak checks for lock-free atomics from [compiler-rt][builtins] Tweak tests for lock-free atomics to [compiler-rt][builtins] Tweak checks for lock-free atomics.
Aug 16 2020, 4:08 PM · Restricted Project
luismarques requested review of D86043: [compiler-rt][builtins] Tweak checks for lock-free atomics.
Aug 16 2020, 3:51 PM · Restricted Project
luismarques requested review of D86036: [compiler-rt][RISCV] Use muldi3 builtin assembly implementation.
Aug 16 2020, 6:16 AM · Restricted Project

Aug 4 2020

luismarques added a comment to D62732: [RISCV] Add SystemV ABI.

As for next steps, if we're happy with the state then I think this should land (assuming qemu is sufficient given it is public), and then we can flesh out other bits which give a better experience. I'm not sure how to connect this to any automated testing, or where to document any way of checking this manually, the state of that isn't quite clear, so any clarity there helps.

Aug 4 2020, 3:21 PM · Restricted Project

Jul 30 2020

luismarques added a comment to D84833: Implement indirect branch generation in position independent code for the RISC-V target.

I _believe_ we need:

isBranch = 1, isTerminator = 1, isBarrier = 1

?

Jul 30 2020, 7:40 AM · Restricted Project
luismarques added a comment to D84833: Implement indirect branch generation in position independent code for the RISC-V target.

Other than the TODO, yes (and hopefully you agree with my suggestions here!). I would also like to see branch-relaxation.ll grow RV64 RUN lines for completeness, even if it's probably a bit redundant (and creates a slight explosion..), but I can do that as a follow-up patch.

Jul 30 2020, 7:24 AM · Restricted Project