Page MenuHomePhabricator

rengolin (Renato Golin)
Toolchain Engineer

Projects

User does not belong to any projects.

User Details

User Since
Oct 19 2012, 12:57 AM (442 w, 5 d)

Recent Activity

Today

rengolin added a comment to D93798: [CSKY 4/n] Add basic CSKYAsmParser and CSKYInstPrinter.

And this one, too: https://reviews.llvm.org/D95030

Wed, Apr 14, 2:41 AM · Restricted Project
rengolin accepted D94007: [CSKY 5/n] Add support for all CSKY basic integer instructions except for branch series.

This looks good to me, thanks!

Wed, Apr 14, 2:39 AM · Restricted Project

Yesterday

rengolin added a comment to D93798: [CSKY 4/n] Add basic CSKYAsmParser and CSKYInstPrinter.

There are still some comments on the remaining patches in this set that need addressing before all of them can be committed as a whole.

Tue, Apr 13, 4:49 AM · Restricted Project

Tue, Mar 16

rengolin added a comment to D98695: [Docs] Suggest mentioning review page in the commit message.

Actually, this is mentioned in a different document: https://www.llvm.org/docs/Phabricator.html#committing-a-change

Tue, Mar 16, 4:59 AM · Restricted Project

Mar 8 2021

rengolin accepted D88391: [M68k] (Patch 5/8) Target lowering.

Kudos to everyone involved.

Mar 8 2021, 2:05 AM · Restricted Project

Mar 4 2021

rengolin added a comment to D95030: [CSKY 7/n] Initial codegen support for ALU operations.

Some comments, mostly on style, but this looks like a fair skeleton lowering to me.

Mar 4 2021, 9:21 AM · Restricted Project
rengolin added a comment to D95029: [CSKY 6/n] Add support branch and symbol series instruction.

+1 to @myhsu's points, plus a few comments, but otherwise, is looking good.

Mar 4 2021, 9:03 AM · Restricted Project
rengolin added a comment to D94007: [CSKY 5/n] Add support for all CSKY basic integer instructions except for branch series.

This looks good to me. @MaskRay any comments?

Mar 4 2021, 8:43 AM · Restricted Project

Feb 16 2021

rengolin added inline comments to D88392: [M68k] (Patch 6/8) IR Tests.
Feb 16 2021, 3:48 AM · Restricted Project
rengolin added a comment to D88391: [M68k] (Patch 5/8) Target lowering.

Any chance we can get this reviewed?

Feb 16 2021, 3:15 AM · Restricted Project

Feb 15 2021

rengolin added a comment to D96632: [THUMB2] add .w suffixes for ldr/str (immediate) T4.
A8.1.3 Instruction encodings

For example, the assembler
syntax documented for the 32-bit Thumb AND (register) encoding includes the .W qualifier to ensure that the
32-bit encoding is selected even for the small proportion of operand combinations for which the 16-bit
encoding is also available.

Which is not a great example because when I tested this with gcc, it would prefer 16 bit encodings in most situations
that I could come up with. However, AND has conditions for being in or out of an IT block so that might be higher priority than any .w encoding.

Feb 15 2021, 4:51 AM · Restricted Project

Feb 4 2021

rengolin added a comment to D95586: [ARM] permit PC as destination of MOVS.

GNU objdump only warns with <UNPREDICTABLE> for the second form (which is the form this patch is concerned with). That seems like good justification for removing the test from the kernel.

Feb 4 2021, 2:46 AM · Restricted Project

Feb 3 2021

rengolin committed rGdd2dac2fd076: Fix MLIR Async Runtime DLL on Windows (authored by mjp41).
Fix MLIR Async Runtime DLL on Windows
Feb 3 2021, 4:24 AM
rengolin closed D95933: Fix MLIR Async Runtime DLL on Windows.
Feb 3 2021, 4:24 AM · Restricted Project
rengolin added a comment to D95386: Fix namespace for MLIR Async Runtime.

It looks like this broke the windows mlir bot. Please revert it or fix it ASAP:

http://lab.llvm.org:8011/#/builders/13/builds/4013

Feb 3 2021, 2:13 AM · Restricted Project

Feb 2 2021

rengolin committed rGb7d80058ff46: Fix namespace for MLIR Async Runtime (authored by mjp41).
Fix namespace for MLIR Async Runtime
Feb 2 2021, 11:18 AM
rengolin closed D95386: Fix namespace for MLIR Async Runtime.
Feb 2 2021, 11:18 AM · Restricted Project
rengolin added a comment to D95386: Fix namespace for MLIR Async Runtime.

LG, my previous comment was more aspirational, I didn't intend to hold this revision :)

Feb 2 2021, 10:13 AM · Restricted Project
rengolin added a comment to D95386: Fix namespace for MLIR Async Runtime.

@rengolin, can you merge as is?

Feb 2 2021, 9:43 AM · Restricted Project

Jan 30 2021

rengolin added a comment to D88390: [M68k] (Patch 4/8) MC layer and object file support.

It's happening what I was hoping and also expecting - external people just coming by and sending such improvements because they love the retro architecture and enjoy working on LLVM.

Jan 30 2021, 5:02 AM · Restricted Project
rengolin added inline comments to D95586: [ARM] permit PC as destination of MOVS.
Jan 30 2021, 5:00 AM · Restricted Project
rengolin added a comment to D95586: [ARM] permit PC as destination of MOVS.

My reading of the manual says this change is correct. All the exceptions and deprecations in MOVs and LSL seem to not be in ARM mode with the S flag set and without the PC in Rm (source register).

Jan 30 2021, 4:57 AM · Restricted Project
rengolin added a comment to D95586: [ARM] permit PC as destination of MOVS.

Then why does llvm/test/MC/Disassembler/ARM/unpredictable-LSL-regform.txt test LSL, and not ARS, LSR, or ROR? (There are other tests llvm/test/MC/Disassembler/ARM/unpredictable-, but there don't seem to be tests for those).

Jan 30 2021, 4:39 AM · Restricted Project

Jan 29 2021

rengolin added a comment to D88390: [M68k] (Patch 4/8) MC layer and object file support.

We don't have AsmParser and disassembler right now. We're using MIR as input to test our integrated assembler, so you're right, we can't test MC layer without CodeGen. But why would that will be a hard blocker?

Jan 29 2021, 7:02 AM · Restricted Project

Jan 28 2021

rengolin added a comment to D95586: [ARM] permit PC as destination of MOVS.

ARS, LSR, ROR have the following warning on the ARM ARM:

Jan 28 2021, 2:27 AM · Restricted Project

Jan 27 2021

rengolin added a comment to D88391: [M68k] (Patch 5/8) Target lowering.

Yes and no. I had a bunch of comments on many of them and will need to make another pass over them to see if there's anything I've missed (or that's been introduced). All accepted means is that someone in the community thinks it's fine and nobody's explicitly rejected it (which I could have done, but I don't like to use the request changes feature as having big red marks over entire patch series can be demoralising). Certainly shouldn't be landed as soon as everything has an approval, rather that should be the point to request that people who have given feedback take another look over the series if they want to.

Jan 27 2021, 12:25 PM · Restricted Project
rengolin added a comment to D95386: Fix namespace for MLIR Async Runtime.

This patch is fine, but I'd still be interested to understand if this could be made to show up during check-mlir with one of the Cmake configuration that can run on Windows?

Jan 27 2021, 11:25 AM · Restricted Project
rengolin added a comment to D95386: Fix namespace for MLIR Async Runtime.

@ezhulenev I think I can see what the fix should be. Does the check-mlir cover this example? Or is there additional testing I should do?

Jan 27 2021, 3:37 AM · Restricted Project

Jan 26 2021

rengolin added a comment to D95386: Fix namespace for MLIR Async Runtime.

This is the error:

Jan 26 2021, 2:53 AM · Restricted Project

Jan 25 2021

rengolin added a comment to D93798: [CSKY 4/n] Add basic CSKYAsmParser and CSKYInstPrinter.

anymore comments or LGTM?

Jan 25 2021, 2:39 AM · Restricted Project

Jan 24 2021

rengolin accepted D95315: [CODE_OWNERS][M68k] (Patch 0/8) Add code owner for the M68k target.
Jan 24 2021, 11:37 AM · Restricted Project

Jan 22 2021

rengolin added a comment to D94451: Proposal for adding Bazel build configuration in-tree with peripheral support.

Overall, I think this is great. It answers all questions I've seen posed on the RFCs and is well aligned with the tiers policy, in text and intent.

Jan 22 2021, 1:59 AM

Jan 20 2021

rengolin added a comment to D93798: [CSKY 4/n] Add basic CSKYAsmParser and CSKYInstPrinter.

Yes, a first batch of patches to reach the point that it can generate first ALU asm codegen. In all, I propose following patches to construct the first batch.

https://reviews.llvm.org/D93798
https://reviews.llvm.org/D94007
https://reviews.llvm.org/D95029
https://reviews.llvm.org/D95030

Jan 20 2021, 6:31 AM · Restricted Project

Jan 19 2021

rengolin added a comment to D93798: [CSKY 4/n] Add basic CSKYAsmParser and CSKYInstPrinter.

Hi Zixuan,

Jan 19 2021, 3:49 AM · Restricted Project
rengolin added a comment to D88385: [TableGen][M68K] (Patch 1/8) Utilities for complex instruction addressing modes: CodeBeads and logical operand helper functions.

The plan is for the entire patch series to be accepted before any are pushed.

Jan 19 2021, 3:44 AM · Restricted Project

Jan 16 2021

rengolin added a comment to D88385: [TableGen][M68K] (Patch 1/8) Utilities for complex instruction addressing modes: CodeBeads and logical operand helper functions.

Absolutely. I agree this problem should be solve before graduate from experimental target. I will create these bugs soon

Jan 16 2021, 5:22 AM · Restricted Project

Jan 15 2021

rengolin accepted D88385: [TableGen][M68K] (Patch 1/8) Utilities for complex instruction addressing modes: CodeBeads and logical operand helper functions.

That sounds reasonable (but +1 to @RKSimon 's comment about reviewing all this "special sauce" complexity before graduating from experimental)

Jan 15 2021, 1:58 PM · Restricted Project
rengolin added a comment to D88385: [TableGen][M68K] (Patch 1/8) Utilities for complex instruction addressing modes: CodeBeads and logical operand helper functions.

Yeah my concern is not about m68k doing weird things, it's about the code that's added in the target-independent places to support m68k that I am most concerned with, as it affects every target regardless of whether m68k is enabled.

Jan 15 2021, 9:30 AM · Restricted Project

Jan 13 2021

rengolin added reviewers for D94557: [ARM] Fixed incorrect lowering when using GNUEABI (libgcc) and 16bit floats: compnerd, peter.smith.
Jan 13 2021, 9:41 AM · Restricted Project
rengolin added a comment to D94451: Proposal for adding Bazel build configuration in-tree with peripheral support.

If this pitch is accepted, then what are the next steps required in order for bazel or another build system to be added to tree? For example, can they automatically be added if they meet the support policy requirements or does there need to be some kind of community ack for? I think the pitch needs to be more clear about what this process is.

Jan 13 2021, 4:07 AM
rengolin added a comment to D94557: [ARM] Fixed incorrect lowering when using GNUEABI (libgcc) and 16bit floats.

Also, please use diff -U9999 as documented: https://llvm.org/docs/Phabricator.html#requesting-a-review-via-the-web-interface

Jan 13 2021, 2:03 AM · Restricted Project

Jan 6 2021

rengolin added a comment to D88385: [TableGen][M68K] (Patch 1/8) Utilities for complex instruction addressing modes: CodeBeads and logical operand helper functions.

Why would refactoring TSFlags into a int-ptr union just for M68k be better than adding the CodeBeads functionality just for M68k?

Jan 6 2021, 3:32 AM · Restricted Project

Jan 5 2021

rengolin added a comment to D92748: [VE][compiler-rt] Add VE as a target of crt.

Thank you for the response. Your description is correct. That is what I tried to describe. Thanks.

Jan 5 2021, 1:39 AM · Restricted Project, Restricted Project
rengolin added a comment to D92748: [VE][compiler-rt] Add VE as a target of crt.

@compnerd , I have a question. I'm thinking that you might misunderstand my explanations. VE is almost merged. I can compile llvm-project using already upstreamed llvm/clang. Now, in order to continue the upstreaming, I need crtbegin.o and crtend.o in compiler-rt. It is a reason why I submit this patch. Didn't you misunderstand the situation? Thanks!

Jan 5 2021, 1:25 AM · Restricted Project, Restricted Project

Dec 31 2020

rengolin added a comment to D93798: [CSKY 4/n] Add basic CSKYAsmParser and CSKYInstPrinter.

I think it's not worthwhile to spend too much time to discuss this open topic. Let's come back to review this and following patches, please. Thank you! I can make few diff files based on this one.

Dec 31 2020, 6:36 AM · Restricted Project

Dec 30 2020

rengolin added a comment to D93798: [CSKY 4/n] Add basic CSKYAsmParser and CSKYInstPrinter.

I think it's early-refactoring issue. We should assume the current patch is good in current codebase context, or we will re-review many times.

Dec 30 2020, 5:31 AM · Restricted Project

Dec 29 2020

rengolin added a comment to D93798: [CSKY 4/n] Add basic CSKYAsmParser and CSKYInstPrinter.

I don't think too many patches on the fly is good to upstream because they influence each other and need change on the way of review.

Dec 29 2020, 3:40 AM · Restricted Project

Dec 21 2020

rengolin added inline comments to D88394: [Driver][M68k] (Patch 8/8) Add driver support for M68k.
Dec 21 2020, 1:02 PM · Restricted Project
rengolin accepted D93372: [CSKY 3/n] Add bare-bones C-SKY MCTargetDesc.

LGTM, thanks!

Dec 21 2020, 1:01 PM · Restricted Project

Dec 18 2020

rengolin accepted D88392: [M68k] (Patch 6/8) IR Tests.

This LGTM now, thanks!

Dec 18 2020, 4:08 AM · Restricted Project
rengolin accepted D88389: [M68k] (Patch 3/8) Basic infrastructures and target description files.

LGTM too, thanks!

Dec 18 2020, 3:54 AM · Restricted Project

Dec 17 2020

rengolin added inline comments to D93372: [CSKY 3/n] Add bare-bones C-SKY MCTargetDesc.
Dec 17 2020, 3:21 AM · Restricted Project

Dec 16 2020

rengolin added a comment to D93372: [CSKY 3/n] Add bare-bones C-SKY MCTargetDesc.

Looks like a standard mock implementation, basic infrastructure. I don't see anything wrong with it but I've also been far from this area for too long, so I'll let other people review it properly and approve.

Dec 16 2020, 4:36 AM · Restricted Project

Dec 10 2020

rengolin accepted D88394: [Driver][M68k] (Patch 8/8) Add driver support for M68k.

LGTM too, thanks!

Dec 10 2020, 3:49 AM · Restricted Project
rengolin added a comment to D88386: [MIR][M68K] (Patch 2/8): Changes on Target-independent MIR part.

This looks good to me but I'll let other people review again and approve. Thanks!

Dec 10 2020, 3:46 AM · Restricted Project
rengolin added a comment to D88391: [M68k] (Patch 5/8) Target lowering.

@craig.topper are you happy with the changes?

Dec 10 2020, 3:44 AM · Restricted Project

Dec 9 2020

rengolin added a comment to D88385: [TableGen][M68K] (Patch 1/8) Utilities for complex instruction addressing modes: CodeBeads and logical operand helper functions.

Does anyone have any more comments? Otherwise I think we can provisionally accept this.

Dec 9 2020, 4:31 AM · Restricted Project
rengolin added a comment to D88391: [M68k] (Patch 5/8) Target lowering.

Hmm, I am just curious about that whether GlobalISEL is mature enough to use as default instruction selection method without implementing SelectionDAG. As a newer backend, I really would like to drop legacy stuff at start if it works.

Dec 9 2020, 4:28 AM · Restricted Project

Dec 4 2020

rengolin added inline comments to D88394: [Driver][M68k] (Patch 8/8) Add driver support for M68k.
Dec 4 2020, 3:09 PM · Restricted Project

Dec 2 2020

rengolin added a comment to D88393: [cfe][M68k] (Patch 7/8) Basic Clang support.

Thanks for the changes. This looks good to me but I'll let others check again and approve.

Dec 2 2020, 4:04 AM · Restricted Project

Nov 30 2020

rengolin accepted D89180: [CSKY 2/n] Add basic tablegen infra for CSKY.

I can't possibly comment on the ISA implementation, but this looks like a standard table-gen set of files to me. They also don't affect anything else outside of CSKY, so LGTM.

Nov 30 2020, 5:55 AM · Restricted Project

Nov 19 2020

rengolin added a comment to D87981: [X86] AMX programming model..

You should at least get @craig.topper's feedback, given this is a significant change in the x86 backend.

Nov 19 2020, 3:52 AM · Restricted Project, Restricted Project

Nov 18 2020

rengolin added a comment to D88389: [M68k] (Patch 3/8) Basic infrastructures and target description files.

Oh, from the discussion on mailing list I thought this is a requirement.

Nov 18 2020, 3:30 AM · Restricted Project
rengolin added inline comments to D88386: [MIR][M68K] (Patch 2/8): Changes on Target-independent MIR part.
Nov 18 2020, 3:19 AM · Restricted Project

Nov 17 2020

rengolin added a comment to D88389: [M68k] (Patch 3/8) Basic infrastructures and target description files.

Is it worth trying to avoid adding/deleting the *Dummy* files in the patch series? @rengolin any thoughts ?

Nov 17 2020, 5:40 AM · Restricted Project
rengolin added inline comments to D88391: [M68k] (Patch 5/8) Target lowering.
Nov 17 2020, 5:36 AM · Restricted Project
rengolin added inline comments to D88394: [Driver][M68k] (Patch 8/8) Add driver support for M68k.
Nov 17 2020, 3:35 AM · Restricted Project
rengolin added inline comments to D88393: [cfe][M68k] (Patch 7/8) Basic Clang support.
Nov 17 2020, 3:31 AM · Restricted Project
rengolin added a comment to D88392: [M68k] (Patch 6/8) IR Tests.

Nice set of tests, well separated, not a lot of pattern-match on the FileCheck side, which is good, asm and object emission.

Nov 17 2020, 3:27 AM · Restricted Project
rengolin added a comment to D88391: [M68k] (Patch 5/8) Target lowering.

I only had a very quick overview and this looks fine.

Nov 17 2020, 3:23 AM · Restricted Project
rengolin added a comment to D88390: [M68k] (Patch 4/8) MC layer and object file support.

A few nits, but this is looking good.

Nov 17 2020, 3:12 AM · Restricted Project
rengolin added inline comments to D88386: [MIR][M68K] (Patch 2/8): Changes on Target-independent MIR part.
Nov 17 2020, 2:58 AM · Restricted Project

Nov 13 2020

rengolin accepted D90956: [clang][SVE] Activate macro `__ARM_FEATURE_SVE_VECTOR_OPERATORS`..

Hi @fpetrogalli, the document is so dense that it took me a while to check the macros and I was still wrong.

Nov 13 2020, 2:02 AM · Restricted Project

Nov 11 2020

rengolin committed rG6fd9e59e1b3a: [mlir] Fix exports in mlir_async_runtime (authored by Paul Lietar <paul@lietar.net>).
[mlir] Fix exports in mlir_async_runtime
Nov 11 2020, 6:12 AM
rengolin closed D91196: [mlir] Fix exports in mlir_async_runtime..
Nov 11 2020, 6:11 AM · Restricted Project

Nov 10 2020

rengolin added reviewers for D91196: [mlir] Fix exports in mlir_async_runtime.: antiagainst, ezhulenev, mehdi_amini.
Nov 10 2020, 11:46 AM · Restricted Project
rengolin committed rG3073cbd2d4c2: [docs] link new support policy from developer policy (authored by rengolin).
[docs] link new support policy from developer policy
Nov 10 2020, 11:41 AM
rengolin closed D91013: [docs] link new support policy from developer policy.
Nov 10 2020, 11:41 AM · Restricted Project
rengolin updated the diff for D91013: [docs] link new support policy from developer policy.

It's looking a bit redundant to me, but I wanted to make sure we have the link on all three sections in case people are given a link (or navigate from the table-of contents) and don't see the sections above.

Nov 10 2020, 4:06 AM · Restricted Project

Nov 7 2020

rengolin requested review of D91013: [docs] link new support policy from developer policy.
Nov 7 2020, 1:19 PM · Restricted Project
rengolin closed D90761: [docs] Adding a Support Policy.

Sorry, forgot the review text on the commit message:
Closed by 25ba6b2bcd1

Nov 7 2020, 1:17 PM · Restricted Project
rengolin committed rG25ba6b2bcd1e: [docs] Adding a Support Policy (authored by rengolin).
[docs] Adding a Support Policy
Nov 7 2020, 1:06 PM
rengolin added a comment to D90761: [docs] Adding a Support Policy.

Thanks Chris! Will do.

Nov 7 2020, 12:50 PM · Restricted Project
rengolin updated the diff for D90761: [docs] Adding a Support Policy.
Nov 7 2020, 3:25 AM · Restricted Project
rengolin added a comment to D90761: [docs] Adding a Support Policy.

LGTM from me on the principle, I didn't get into detailed wordsmith. Overall I would have been more concise and lay down the general principle rather than trying to prescribe this level of details (like the "Reinstatement" section for example), I believe that's also more in line with how we've been operating / documenting things in the past, but I don't really mind it either.
I suspect we can incrementally fix/improve anything as we go as well.

Nov 7 2020, 2:55 AM · Restricted Project
rengolin updated the diff for D90761: [docs] Adding a Support Policy.
  • Include a short phrase on inclusion policy, as requested.
  • Fixed some typos
Nov 7 2020, 2:50 AM · Restricted Project
rengolin added inline comments to D90761: [docs] Adding a Support Policy.
Nov 7 2020, 2:49 AM · Restricted Project

Nov 6 2020

rengolin accepted D90956: [clang][SVE] Activate macro `__ARM_FEATURE_SVE_VECTOR_OPERATORS`..

Simple and straightforward, with documentation! :)

Nov 6 2020, 9:57 AM · Restricted Project
rengolin accepted D90950: [AArch64]Add memory op cost model for SVE.

Nice change, simpler to read and easier to get right. LGTM, thanks!

Nov 6 2020, 8:40 AM · Restricted Project
rengolin added a comment to D90761: [docs] Adding a Support Policy.

Sorry, s/George/Geoffrey/. :S

Nov 6 2020, 2:43 AM · Restricted Project
rengolin updated the diff for D90761: [docs] Adding a Support Policy.
  • Adding inclusion policy section
  • Reword phrase in deprecation policy
Nov 6 2020, 2:41 AM · Restricted Project
rengolin added a comment to D90761: [docs] Adding a Support Policy.

Overall LGTM, with some wording nits. One thing I think is missing is guidance for how a peripheral component gets added. Experimental targets already have their own documentation, and my impression is that editor bindings are just a matter of sending a patch (which seems reasonable to me). What about something like:

To add a new peripheral component not covered under an existing policy, send an RFC to the appropriate dev list proposing its addition and explaining how it will meet the support requirements listed here.

Nov 6 2020, 2:41 AM · Restricted Project

Nov 5 2020

rengolin updated the diff for D90761: [docs] Adding a Support Policy.

Rewriting some confusing paragraphs.

Nov 5 2020, 10:07 AM · Restricted Project
rengolin added inline comments to D90761: [docs] Adding a Support Policy.
Nov 5 2020, 10:06 AM · Restricted Project
rengolin added inline comments to D90761: [docs] Adding a Support Policy.
Nov 5 2020, 9:52 AM · Restricted Project
rengolin updated the diff for D90761: [docs] Adding a Support Policy.

Updating simple suggestions / fixups.

Nov 5 2020, 9:51 AM · Restricted Project

Nov 4 2020

rengolin updated the diff for D90761: [docs] Adding a Support Policy.
  • Adding page link to "Getting Involved" page.
  • Fixing some formatting errors (Sphinx reports no warnings on my box).
Nov 4 2020, 9:10 AM · Restricted Project
rengolin updated the diff for D90761: [docs] Adding a Support Policy.
Nov 4 2020, 8:43 AM · Restricted Project
rengolin added inline comments to D90761: [docs] Adding a Support Policy.
Nov 4 2020, 8:42 AM · Restricted Project
rengolin updated the diff for D90761: [docs] Adding a Support Policy.
Nov 4 2020, 5:48 AM · Restricted Project