rengolin (Renato Golin)
Toolchain Engineer

Projects

User does not belong to any projects.

User Details

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

Recent Activity

Yesterday

rengolin accepted D33120: [ARM] Add VLDx/VSTx sched defs for machine-schedulers. NFCI..

Ok, I think it's good as it is. Looking forward to more patches in this area. :)

Tue, May 23, 9:08 AM
rengolin accepted D33205: ARM] Fix Neon vector type alignment to 64-bit.

Approving, so that when Steve does the same, the review is free to commit.

Tue, May 23, 9:07 AM
rengolin added a comment to D33205: ARM] Fix Neon vector type alignment to 64-bit.

Hi all:
I have made the changes (excluding Android) as recommended. Please have a look if it is now fine.

Tue, May 23, 9:06 AM
rengolin added a comment to D33120: [ARM] Add VLDx/VSTx sched defs for machine-schedulers. NFCI..

Hi Javed,

Tue, May 23, 5:44 AM

Mon, May 22

rengolin added a comment to D32871: [LV] Using VPlan to model the vectorized code and drive its transformation.

Some initial comments...

Mon, May 22, 10:44 AM
rengolin added reviewers for D32166: Improve LoopVectorizers estimation of scalar loops by predicting LSR behaviour: mssimpso, mkuper.

In this sense, isn't the loop vectorizer run too early?

Mon, May 22, 10:39 AM
rengolin added inline comments to D33205: ARM] Fix Neon vector type alignment to 64-bit.
Mon, May 22, 9:30 AM
rengolin added a comment to D33205: ARM] Fix Neon vector type alignment to 64-bit.

Ah, I just found the email thread, looks like it never reached phabricator. Sorry for the confusion.

Mon, May 22, 9:28 AM
rengolin requested changes to D33205: ARM] Fix Neon vector type alignment to 64-bit.

Wait, Stephen had a concern, we need his feedback on my reply before approving.

Mon, May 22, 9:20 AM
rengolin added a comment to D32871: [LV] Using VPlan to model the vectorized code and drive its transformation.

Hi Ayal, sorry for the delay. Can you commit the plan doc separately?

Mon, May 22, 2:42 AM

Sun, May 21

rengolin added a comment to D32737: [Constants][SVE] Represent the runtime length of a scalable vector.

Put differently, this patch makes sense *once* we clearly have consensus on llvm-dev. So far, the only thread I can find did not reach any meaningful consensus. Notably, none of the code generator people were heavily contributing to that thread, and there remain large unmentioned technical concerns (stack spills, alloca size, etc)

Sun, May 21, 3:41 PM
rengolin added a comment to D32737: [Constants][SVE] Represent the runtime length of a scalable vector.

As I explained to Hal on his comment, that is correct but doesn't have the effect you're expecting.

Which comment? FWIW, I didn't see a particular response.

Sun, May 21, 1:17 PM
rengolin added a comment to D32737: [Constants][SVE] Represent the runtime length of a scalable vector.

Just to be clear, I'm not *against* the idea of an intrinsic, nor I'm pushing this patch for any personal/professional agenda. I hope I have made that perfectly clear on my previous reviews on the same patches before.

Sun, May 21, 11:02 AM
rengolin added a comment to D32737: [Constants][SVE] Represent the runtime length of a scalable vector.

From my point of view, making scalable vectors a native and core type of IR is the only way forward, because the semantics needs to be ingrained in the language to make sense. AFAICS, at the IR level, the differences between vectors and scalable ones is not that big a deal, certainly not bigger than vectors versus scalar.

This is much more up for debate. Details matter. Compilers are a set of engineering tradeoffs. You need to explain and defend your position carefully, not just state it as though it were "obviously true".

Sun, May 21, 10:51 AM
rengolin added a comment to D32737: [Constants][SVE] Represent the runtime length of a scalable vector.

I've only lightly read the spec, but it looks like the vector length can be controlled by writing to the ZCR_ELn registers (so, e.g. user code could make a syscall to change the vector length)?

Sun, May 21, 4:52 AM
rengolin added inline comments to D32737: [Constants][SVE] Represent the runtime length of a scalable vector.
Sun, May 21, 4:38 AM
rengolin added a comment to D32737: [Constants][SVE] Represent the runtime length of a scalable vector.

I agree with Chandler on this. Making something a first class type in LLVM has wide reaching effects, including requiring the ability to load/store/phi the value, pass it by argument, etc.

Sun, May 21, 4:32 AM

Sat, May 20

rengolin added reviewers for D32737: [Constants][SVE] Represent the runtime length of a scalable vector: jmolloy, kristof.beyls, aadg, hfinkel, aschwaighofer, mssimpso, Ayal, mkuper.
Sat, May 20, 3:17 PM
rengolin added reviewers for D32737: [Constants][SVE] Represent the runtime length of a scalable vector: delena, asb.

Uh, where was it discussed? It's entirely possible I missed it, but I can't find any consensus on any of the threads that we actually want to support runtime vector width in LLVM's IR.

Sat, May 20, 3:08 PM
rengolin requested changes to D32833: [Triple] Add method for triple canonicalization.

Right, I think this is a useful thing to have, but it needs a better plan.

Sat, May 20, 1:10 PM

Fri, May 19

rengolin added a comment to D32737: [Constants][SVE] Represent the runtime length of a scalable vector.

@echristo @chandlerc @lattner @majnemer Ping.

Fri, May 19, 5:09 AM

Thu, May 18

rengolin added a comment to D33128: [AsmParser] Mnemonic Spell Corrector.

Hi Sjoerd,

Thu, May 18, 6:34 AM

Wed, May 17

rengolin added a comment to D33120: [ARM] Add VLDx/VSTx sched defs for machine-schedulers. NFCI..

Hi Javed,

Wed, May 17, 8:37 AM
rengolin added a comment to D32422: LoopVectorizer: let target prefer scalar addressing computations (+ minor improvements in SystemZTTI).

Right, I see what you mean about the "address computation" to be forced into scalar registers. Does that mean that you have to use the same GR to load into all lanes of the vector, so supposedly load+increment+load+increment...?

It doesn't have to be the same GR for all the lanes, but it must be a GR (since we don't use the slow "element gather/scatter")

Regardless, between iterations of the loop, you want to keep the addresses in the GRs throughout the execution of the loop. Is that correct?

Yes, that's better than having to extract elements. The extracts are relatively expensive.

Wed, May 17, 8:33 AM
rengolin added reviewers for D32422: LoopVectorizer: let target prefer scalar addressing computations (+ minor improvements in SystemZTTI): delena, jmolloy.

I don't quite follow how this has to do with vector shuffles...? On SystemZ, all addresses residing in vectors must be extracted before use. (There are slow vector element gather/scatter, which are not used by the backend, so they are irrelevant).

Wed, May 17, 7:50 AM
rengolin added a comment to D32422: LoopVectorizer: let target prefer scalar addressing computations (+ minor improvements in SystemZTTI).

Hi Jonas,

Wed, May 17, 7:02 AM

Tue, May 16

rengolin committed rL303193: Revert "[ARM] Mark LEApcrel instructions as isAsCheapAsAMove".
Revert "[ARM] Mark LEApcrel instructions as isAsCheapAsAMove"
Tue, May 16, 11:12 AM
rengolin committed rL303146: Merging rr302639:.
Merging rr302639:
Tue, May 16, 12:10 AM

Sat, May 13

rengolin accepted D32605: Recognize CTLZ builtin.

Perfect, thanks! LGTM.

Sat, May 13, 3:32 AM

Fri, May 12

rengolin added inline comments to D32605: Recognize CTLZ builtin.
Fri, May 12, 1:42 PM
rengolin added a comment to D32605: Recognize CTLZ builtin.

Right, just to make sure it's doing what we expect it to, did you try to run this with targets "armv7a" and "armv4t"? They should have different results.

Fri, May 12, 1:13 AM

Thu, May 11

rengolin added a comment to D30324: [ARM] Thumb2: favor R4-R7 over R12/LR in allocation order when opt for minsize.

Hi Weiming,

Thu, May 11, 11:52 AM
rengolin committed rL302786: [MSAN] test failed randomly on ARM when XFAILED for MIPS.
[MSAN] test failed randomly on ARM when XFAILED for MIPS
Thu, May 11, 4:19 AM

Wed, May 10

rengolin accepted D32792: [LLVM][inline-asm] Altmacro string escape character '!'.

As parsers go, simplicity and elegance are usually mutually exclusive.

Wed, May 10, 3:06 AM
rengolin added a comment to D32605: Recognize CTLZ builtin.

That's not true. ARMv4 for example has no clz, it's a V5T feature.

Wed, May 10, 1:22 AM

Tue, May 9

rengolin added a comment to D32605: Recognize CTLZ builtin.

I agree. If the CPU has it, it will be beneficial. If it doesn't, it is only a useful transformation if the intrinsic can be constant folded.

Tue, May 9, 1:07 PM
rengolin added a comment to D31801: Performance enhancements for Cavium ThunderX2 T99.

Ping2!! :-)

Tue, May 9, 12:36 PM
rengolin added a comment to D32605: Recognize CTLZ builtin.

Wait, we also need the TTI callbacks. I thought you were going to introduce them.

Tue, May 9, 12:32 PM
rengolin committed rL302520: [Dwarf] Disable reference verification for now (PR32972).
[Dwarf] Disable reference verification for now (PR32972)
Tue, May 9, 5:50 AM

Mon, May 8

rengolin added a comment to D30086: Add generic IR vector reductions.

When people add new intrinsics they don't really add tests to check the IR since you get assertion failures during the FunctionType creation if something goes wrong.

Mon, May 8, 6:04 AM
rengolin added a comment to D30086: Add generic IR vector reductions.

Right, this is looking much better. Now, what about tests?

Mon, May 8, 5:43 AM
rengolin added a comment to D30086: Add generic IR vector reductions.

Hi Amara,

Mon, May 8, 5:27 AM

Fri, May 5

rengolin added a comment to D32833: [Triple] Add method for triple canonicalization.

Also, where would this be used?

Fri, May 5, 7:39 AM
rengolin added inline comments to D32833: [Triple] Add method for triple canonicalization.
Fri, May 5, 7:29 AM
rengolin accepted D32534: [ARM] Add support for ORR and ORN instruction substitutions.

Sorry for the delay, LGTM. Thanks!

Fri, May 5, 3:26 AM
rengolin added a comment to D27620: [Assembler] Report multiple near misses for invalid instructions.

@echristo, any comments on this?

Fri, May 5, 3:24 AM
rengolin added inline comments to D32792: [LLVM][inline-asm] Altmacro string escape character '!'.
Fri, May 5, 3:22 AM

Thu, May 4

rengolin committed rL302165: [test-release] Status update *before* long gzip.
[test-release] Status update *before* long gzip
Thu, May 4, 9:34 AM
rengolin accepted D32850: [ArgPromotion] Fix a truncated variable.

Right, makes sense. This is an obvious fix, so I feel comfortable approving this one. LGTM. Thanks!

Thu, May 4, 3:15 AM
rengolin added inline comments to D32200: [LV] Refactor ILV.vectorize[Loop]() by introducing LVP.executePlan().
Thu, May 4, 2:39 AM
rengolin added a comment to D32850: [ArgPromotion] Fix a truncated variable.

Wouldn't it be better to set the type manually to something like uint32_t or uint64_t? To make sure they're the same across all arches?

Thu, May 4, 2:25 AM
rengolin accepted D32701: [LLVM][inline-asm][Altmacor] Altmacro string delimiter '<..>'.

LGTM. Thanks!

Thu, May 4, 2:09 AM
rengolin added a comment to D31801: Performance enhancements for Cavium ThunderX2 T99.

I'm ok with the change, as long as @javed.absar is happy. :)

Thu, May 4, 1:36 AM

Wed, May 3

rengolin accepted D32780: [AArch64] Make the TargetParser add CPU exts provided by the arch..

I really thought it was there already! Thanks for the patch! LGTM.

Wed, May 3, 10:45 AM
rengolin added a comment to D32701: [LLVM][inline-asm][Altmacor] Altmacro string delimiter '<..>'.

You seem to have lost the negative test.

Wed, May 3, 7:31 AM
rengolin added a comment to D32701: [LLVM][inline-asm][Altmacor] Altmacro string delimiter '<..>'.

With that in mind, I think you should then go back to your original implementation (ignore quotes) and let the expression evaluation crash if there's any single quote in between (or whatever is the expected behaviour).

Wed, May 3, 7:14 AM
rengolin added inline comments to D32701: [LLVM][inline-asm][Altmacor] Altmacro string delimiter '<..>'.
Wed, May 3, 4:07 AM
rengolin added reviewers for D32737: [Constants][SVE] Represent the runtime length of a scalable vector: chandlerc, echristo, lattner, majnemer.

Hi Graham,

Wed, May 3, 3:46 AM

Tue, May 2

rengolin added a comment to D32166: Improve LoopVectorizers estimation of scalar loops by predicting LSR behaviour.

Hi Jonas,

Tue, May 2, 12:36 PM
rengolin added a comment to D30086: Add generic IR vector reductions.

Ok, so I've restructured the two functions a bit so that the simple (non minmax) reductions are generated from the createSimpleTargetReduction() function and the recurrence descriptor uses that for the simple cases, passing in the opcode.

Tue, May 2, 11:12 AM
rengolin added inline comments to D32737: [Constants][SVE] Represent the runtime length of a scalable vector.
Tue, May 2, 11:05 AM
rengolin added inline comments to D30086: Add generic IR vector reductions.
Tue, May 2, 4:41 AM
rengolin added inline comments to D32701: [LLVM][inline-asm][Altmacor] Altmacro string delimiter '<..>'.
Tue, May 2, 4:25 AM
rengolin added inline comments to D30086: Add generic IR vector reductions.
Tue, May 2, 4:06 AM

Mon, May 1

rengolin accepted D32526: [LLVM][inline-asm] Altmacro absolute expression '%' feature.

Thanks! LGTM with the test change.

Mon, May 1, 4:05 AM
rengolin added a comment to D32526: [LLVM][inline-asm] Altmacro absolute expression '%' feature.

Can you merge D32523 with this one?

Mon, May 1, 3:06 AM

Fri, Apr 28

rengolin added a comment to D32605: Recognize CTLZ builtin.

Yes.

Fri, Apr 28, 9:12 AM
rengolin added a comment to D31801: Performance enhancements for Cavium ThunderX2 T99.

So, then, this would be a case of "some micro-architectures can do this for any floating-point type, some others need special treatment".

Fri, Apr 28, 7:09 AM
rengolin added a comment to D31801: Performance enhancements for Cavium ThunderX2 T99.

Right, I clicked the link and it took me to the old code. :)

Fri, Apr 28, 6:51 AM
rengolin added a comment to D32200: [LV] Refactor ILV.vectorize[Loop]() by introducing LVP.executePlan().

Hi Ayal,

Fri, Apr 28, 3:28 AM
rengolin added inline comments to D31801: Performance enhancements for Cavium ThunderX2 T99.
Fri, Apr 28, 2:44 AM
rengolin added a comment to D32460: ADT: handle special case of ARM environment for SuSE.

Yes, the idea is that the textual IR uses the correct triple, unless it is overidden at the command line.

Why not just handle this in clang?

Fri, Apr 28, 2:17 AM
rengolin added a comment to D32605: Recognize CTLZ builtin.

Guarding is easier. Let's do this only if someone complains about performance.

Fri, Apr 28, 2:02 AM

Thu, Apr 27

rengolin added a comment to D32605: Recognize CTLZ builtin.

In this case builtin should be expanded at codegen in the most profitable way.

Thu, Apr 27, 12:13 PM
rengolin added a comment to D28975: [LV] Introducing VPlan to model the vectorized code and drive its transformation.

So, I think the document is so good and can be very helpful in further communications, that I think it should be committed separately.

Thu, Apr 27, 12:10 PM
rengolin added a comment to D32605: Recognize CTLZ builtin.

Hi Evgeny,

Thu, Apr 27, 11:59 AM

Wed, Apr 26

rengolin added inline comments to D32526: [LLVM][inline-asm] Altmacro absolute expression '%' feature.
Wed, Apr 26, 8:17 AM
rengolin added reviewers for D32530: [SVE] Scalable Vector IR Type: echristo, chandlerc, hfinkel.

Hi Graham, thanks for this work.

Wed, Apr 26, 5:24 AM
rengolin added a comment to D32526: [LLVM][inline-asm] Altmacro absolute expression '%' feature.

I think this patch and D32523 are small and simple enough that it could be merged into one...

Wed, Apr 26, 4:07 AM
rengolin added a reviewer for D32296: [ARM] Modify lowering of setjmp: t.p.northover.

Adding Tim, as this is likely going to affect Darwin more than Linux.

Wed, Apr 26, 3:26 AM
rengolin added a comment to D32247: Switch AArch64 to use reduction intrinsics.

As soon as the dependency patch is approved, this one looks ok.

Wed, Apr 26, 3:23 AM
rengolin added a comment to D32523: [LLVM][Inline-ASM] Adding Altmacro directive to LLVM .

Well, for now, this isn't doing anything other than silently ignoring the directives.

Wed, Apr 26, 3:07 AM

Tue, Apr 25

rengolin added a comment to D32460: ADT: handle special case of ARM environment for SuSE.

This could have side-effects, too, especially when going through textual IR, no?

Tue, Apr 25, 1:44 AM

Mon, Apr 24

rengolin added inline comments to D30086: Add generic IR vector reductions.
Mon, Apr 24, 8:59 AM
rengolin added inline comments to D30086: Add generic IR vector reductions.
Mon, Apr 24, 8:52 AM
rengolin added inline comments to D30086: Add generic IR vector reductions.
Mon, Apr 24, 8:25 AM
rengolin committed rL301176: [DWARF] Move test to x86 directory.
[DWARF] Move test to x86 directory
Mon, Apr 24, 5:50 AM
rengolin added reviewers for D32427: Fix float abi for SUSE ARM triples: compnerd, rovka, joerg.

I'm not sure this will work at all. Not because it doesn't make sense (it does), but because of several bugs in the soft vs. hard float implementation in the Triple related classes all the way to the back-end.

Mon, Apr 24, 5:47 AM
rengolin accepted D32426: Add SUSE vendor.

LGTM. Thanks!

Mon, Apr 24, 4:27 AM
rengolin added a comment to D32426: Add SUSE vendor.

We have some unit tests for Triple. Can you add some, pls?

Mon, Apr 24, 4:18 AM

Apr 23 2017

rengolin committed rL301111: Revert "[APInt] Fix a few places that use APInt::getRawData to operate within….
Revert "[APInt] Fix a few places that use APInt::getRawData to operate within…
Apr 23 2017, 5:28 AM
rengolin added a reverting commit for rL4: LLVM initial checkin: rL301111: Revert "[APInt] Fix a few places that use APInt::getRawData to operate within….
Apr 23 2017, 5:28 AM
rengolin added a reverting commit for rL1: New repository initialized by cvs2svn.: rL301111: Revert "[APInt] Fix a few places that use APInt::getRawData to operate within….
Apr 23 2017, 5:28 AM
rengolin added a reverting commit for rL3: This commit was manufactured by cvs2svn to create branch 'llvm'.: rL301111: Revert "[APInt] Fix a few places that use APInt::getRawData to operate within….
Apr 23 2017, 5:28 AM
rengolin committed rL301110: Revert "[APInt] Add ashrInPlace method and implement ashr using it. Also fix a….
Revert "[APInt] Add ashrInPlace method and implement ashr using it. Also fix a…
Apr 23 2017, 5:15 AM

Apr 21 2017

rengolin accepted D32347: Add support for openSUSE ARM Triples.

Ok, I see no harm in letting you work upstream, since no one should be relying on the correct behaviour on opensuse for now. LGTM. Thanks!

Apr 21 2017, 5:32 AM
rengolin added reviewers for D32347: Add support for openSUSE ARM Triples: rovka, compnerd.

Yes openSUSE prefers the "hl" prefix to mean HardFloat.

Apr 21 2017, 5:15 AM
rengolin added a comment to D32347: Add support for openSUSE ARM Triples.

Nothing new here, pretty much standard. No "gnueabihf"?

Apr 21 2017, 5:09 AM
rengolin added a comment to D31081: [ARM] ScheduleDAGRRList::DelayForLiveRegsBottomUp must consider OptionalDefs.

I agree with James, refactoring OptionalDef is a task too big for this patch.

Apr 21 2017, 3:40 AM

Apr 20 2017

rengolin added a comment to D32282: [ARM] ACLE Chapter 9 support.

Perfect, thank you! LGTM.

Apr 20 2017, 5:25 AM