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 (475 w, 5 d)

Recent Activity

Yesterday

rengolin accepted D113247: [VE] Make VE official.

This has been proposed on llvm-dev for almost a month and has only attracted two reviews (me and @reames) and I believe all raised points were addressed, including a now consistently green and fast buildbot.

Tue, Nov 30, 2:43 AM · Restricted Project, Restricted Project

Mon, Nov 29

rengolin added a comment to D111460: [X86][LoopVectorize] "Fix" `X86TTIImpl::getAddressComputationCost()`.

I can't predict if the new shuffle patterns will be better or worse on each affected platform, least of all looking at IR, not ASM.

Mon, Nov 29, 10:09 AM · Restricted Project

Thu, Nov 25

rengolin accepted D107519: [AIX][test-suite] Update the telecomm-FFT benchmark to use saved sin/cos values for some inputs..

Hi @rengolin :)

I thought about this again and just wanted to make sure that I did not misunderstand your previous comment.

So, if this also works for in another architecture (like x86_64 or Arm on Linux or Mac), then it should be fine to merge.

Do you mean that it would be preferable for other targets to test this patch as well to ensure it is a reasonable fix? Would this be necessary as I'm using a hard-coding a few of the Linux libm produced values (that produces what is already expected in MultiSource/Benchmarks/MiBench/telecomm-FFT/telecomm-fft.reference_output)?

Thu, Nov 25, 12:53 PM · Restricted Project

Tue, Nov 23

rengolin added inline comments to D95114: HowToReleaseLLVM: Add annual release schedule template.
Tue, Nov 23, 6:22 AM · Restricted Project

Mon, Nov 22

rengolin added inline comments to D114325: Add a best practice section on how to configure a fast builder.
Mon, Nov 22, 11:30 AM · Restricted Project
rengolin added inline comments to D114325: Add a best practice section on how to configure a fast builder.
Mon, Nov 22, 10:56 AM · Restricted Project
rengolin added a comment to D114325: Add a best practice section on how to configure a fast builder.

@reames thanks for the changes, they look good to me.

Mon, Nov 22, 10:06 AM · Restricted Project
rengolin added inline comments to D114325: Add a best practice section on how to configure a fast builder.
Mon, Nov 22, 10:05 AM · Restricted Project

Sun, Nov 21

rengolin added a comment to D114325: Add a best practice section on how to configure a fast builder.

Overall looks good, thanks for writing this!

Sun, Nov 21, 5:57 AM · Restricted Project

Tue, Nov 9

rengolin added a comment to D107519: [AIX][test-suite] Update the telecomm-FFT benchmark to use saved sin/cos values for some inputs..

I was wondering if it makes sense to implement a look-up value approach, and substitute in the Linux libm results whenever the inputs are (pi/4) or (pi/16), respectively.
Furthermore, perhaps this change can be guarded to run for AIX specifically, if that’s a good idea. I’ve updated the revision to illustrate what I mean. I'd appreciate it if you could let me know if you have any thoughts on this.

Tue, Nov 9, 6:59 AM · Restricted Project

Mon, Nov 8

rengolin added a comment to D113247: [VE] Make VE official.

Does this need to come after D113093?

Mon, Nov 8, 1:22 AM · Restricted Project, Restricted Project

Thu, Nov 4

rengolin accepted D111358: TargetLibraryInfo checker tool.

It's much clearer now, thanks for the updates!

Thu, Nov 4, 9:40 AM · Restricted Project
rengolin accepted D112919: [CSKY] Add CSKY 16-bit instruction format and encoding.

LGTM, thanks!

Thu, Nov 4, 5:11 AM · Restricted Project

Wed, Nov 3

rengolin added inline comments to D112919: [CSKY] Add CSKY 16-bit instruction format and encoding.
Wed, Nov 3, 2:36 AM · Restricted Project

Tue, Nov 2

rengolin added inline comments to D112919: [CSKY] Add CSKY 16-bit instruction format and encoding.
Tue, Nov 2, 4:03 AM · Restricted Project

Mon, Nov 1

rengolin added a comment to rGa70a5636a8a4: [docs] Expand a bit on the basics of a buildbot policy.
Mon, Nov 1, 11:33 AM
rengolin added inline comments to D112919: [CSKY] Add CSKY 16-bit instruction format and encoding.
Mon, Nov 1, 11:02 AM · Restricted Project

Oct 29 2021

rengolin accepted D112750: [SparcISelLowering] avoid emitting libcalls to __muloti4 and __mulodi4.

Just like the others, LGTM. Thanks!

Oct 29 2021, 8:02 AM · Restricted Project

Oct 26 2021

rengolin accepted D112206: [CSKY] First patch to construct codegen infra and generate first add instruction.

Thanks again, this looks good to me now. Please wait a day or so to merge, in case there are any more reviews.

Oct 26 2021, 3:00 AM · Restricted Project

Oct 22 2021

rengolin added a comment to D112206: [CSKY] First patch to construct codegen infra and generate first add instruction.

Some general comments but this looks good as a first lowering of simple functions.

Oct 22 2021, 8:30 AM · Restricted Project

Oct 18 2021

rengolin accepted D111701: [CSKY] Complete to add basic integer instruction set.

Just wait another day to make sure no one else has more questions.

Oct 18 2021, 2:00 AM · Restricted Project

Oct 15 2021

rengolin added inline comments to D111701: [CSKY] Complete to add basic integer instruction set.
Oct 15 2021, 4:39 AM · Restricted Project
rengolin added inline comments to D111701: [CSKY] Complete to add basic integer instruction set.
Oct 15 2021, 3:48 AM · Restricted Project

Oct 14 2021

rengolin added a comment to D111701: [CSKY] Complete to add basic integer instruction set.

I have a few comments, but otherwise, it's looking good. Thanks!

Oct 14 2021, 3:08 PM · Restricted Project

Oct 11 2021

rengolin added a comment to D111358: TargetLibraryInfo checker tool.

Hi Paul, thanks for submitting this, it looks super cool.

Oct 11 2021, 9:33 AM · Restricted Project

Oct 4 2021

rengolin added a comment to D110830: [AArch64] Make -mcpu=generic schedule for an in-order core.

This sounds good to me too.

Oct 4 2021, 3:17 AM · Restricted Project

Sep 2 2021

rengolin accepted D108926: [MipsISelLowering] avoid emitting libcalls to __multi3.

LGTM, thanks!

Sep 2 2021, 10:25 AM · Restricted Project

Aug 31 2021

rengolin added inline comments to D108928: [X86ISelLowering] avoid emitting libcalls to __mulodi4().
Aug 31 2021, 2:45 PM · Restricted Project

Aug 30 2021

rengolin added a comment to D108926: [MipsISelLowering] avoid emitting libcalls to __multi3.

Why is multi3 a problem for MIPS but not Arm or x64_32?

Aug 30 2021, 11:19 AM · Restricted Project

Aug 27 2021

rengolin accepted D108844: [MipsISelLowering] avoid emitting libcalls to __mulodi4().

LGTM, thanks!

Aug 27 2021, 3:03 PM · Restricted Project
rengolin accepted D108842: [ARMISelLowering] avoid emitting libcalls to __mulodi4().

LGTM, thanks!

Aug 27 2021, 3:03 PM · Restricted Project
rengolin added a comment to D108844: [MipsISelLowering] avoid emitting libcalls to __mulodi4().

Can you return the flag like you did for ARM? Just to make sure we don't elide everything and then the test is a NOOP

Aug 27 2021, 2:59 PM · Restricted Project
rengolin added a comment to D108844: [MipsISelLowering] avoid emitting libcalls to __mulodi4().

Same comments as the ARM version from @craig.topper apply here, too.

Aug 27 2021, 2:16 PM · Restricted Project
rengolin added a comment to D108842: [ARMISelLowering] avoid emitting libcalls to __mulodi4().

Huh! This was way simpler than I thought! Does that fix the kernel issue?

Aug 27 2021, 2:15 PM · Restricted Project

Aug 19 2021

rengolin committed rG894ad26bd55f: Update {Small}BitVector size_type definition (authored by rengolin).
Update {Small}BitVector size_type definition
Aug 19 2021, 3:14 AM
rengolin closed D108290: Update {Small}BitVector size_type definition.
Aug 19 2021, 3:13 AM · Restricted Project

Aug 18 2021

rengolin added a comment to D108290: Update {Small}BitVector size_type definition.

The last clang-tidy warning (capacity_in_bytes) is pertinent but not for this patch.

Aug 18 2021, 8:18 AM · Restricted Project
rengolin updated the diff for D108290: Update {Small}BitVector size_type definition.

Update with clang-format and clang-tidy hints.

Aug 18 2021, 6:20 AM · Restricted Project
rengolin updated the summary of D108290: Update {Small}BitVector size_type definition.
Aug 18 2021, 4:23 AM · Restricted Project
rengolin updated the diff for D108290: Update {Small}BitVector size_type definition.
Aug 18 2021, 4:21 AM · Restricted Project
rengolin requested review of D108290: Update {Small}BitVector size_type definition.
Aug 18 2021, 4:19 AM · Restricted Project

Aug 17 2021

rengolin added a comment to D108124: Fix type in DenseMap<SmallBitVector, *> to match V.size().

Not sure why my differential comment didn't pick up, but this was closed by a19747ea739

You created a new branch on the LLVM Github repo and I think that is confusing Phabricator: https://github.com/llvm/llvm-project/tree/size_t-warning

Aug 17 2021, 5:10 AM · Restricted Project
rengolin closed D108124: Fix type in DenseMap<SmallBitVector, *> to match V.size().

Not sure why my differential comment didn't pick up, but this was closed by a19747ea739

Aug 17 2021, 3:23 AM · Restricted Project
rengolin added a comment to D108124: Fix type in DenseMap<SmallBitVector, *> to match V.size().

Not the bravest version of the change, but obviously LGTM too ;-)

Aug 17 2021, 3:21 AM · Restricted Project

Aug 16 2021

rengolin added a comment to D108124: Fix type in DenseMap<SmallBitVector, *> to match V.size().

This is the warning:

Aug 16 2021, 7:30 AM · Restricted Project
rengolin updated the diff for D108124: Fix type in DenseMap<SmallBitVector, *> to match V.size().
Aug 16 2021, 7:13 AM · Restricted Project
rengolin added a comment to D108124: Fix type in DenseMap<SmallBitVector, *> to match V.size().

Nope, user code all over LLVM is expecting one to return size_t and the other unsigned, so I'll leave that major refactory to another day.

Aug 16 2021, 7:12 AM · Restricted Project
rengolin added a comment to D108124: Fix type in DenseMap<SmallBitVector, *> to match V.size().

Or, being conservatively (with container sizes), we can use size_type size() on both cases, because that's what the typedef seems to imply in the first place.

Aug 16 2021, 6:44 AM · Restricted Project
rengolin added a comment to D108124: Fix type in DenseMap<SmallBitVector, *> to match V.size().

SmallBitVector size method indeed return a size_t, but in a great show of consistency, BitVector doesn't (it returns size_type which aliases to unsigned)

Aug 16 2021, 6:30 AM · Restricted Project
rengolin retitled D108124: Fix type in DenseMap<SmallBitVector, *> to match V.size() from Fix type in DenseMap<BitVector, *> to math for V.size() to Fix type in DenseMap<BitVector, *> to match V.size().
Aug 16 2021, 5:19 AM · Restricted Project
rengolin requested review of D108124: Fix type in DenseMap<SmallBitVector, *> to match V.size().
Aug 16 2021, 5:18 AM · Restricted Project

Aug 5 2021

rengolin added a comment to D107519: [AIX][test-suite] Update the telecomm-FFT benchmark to use saved sin/cos values for some inputs..

Hi Amy,

Aug 5 2021, 3:11 AM · Restricted Project

Jun 30 2021

rengolin accepted D104935: Update the Polybench tests to check relative error.

I agree a generic comparison library is outside the scope of this patch.

Jun 30 2021, 8:07 AM

Jun 27 2021

rengolin added a comment to D104935: Update the Polybench tests to check relative error.

Hi Andrew,

Jun 27 2021, 3:59 AM

Jun 25 2021

rengolin added inline comments to D104935: Update the Polybench tests to check relative error.
Jun 25 2021, 2:04 PM

Jun 23 2021

rengolin added a comment to D104264: Fix atomic loads and stores of 64-bit values in non-default address spaces on ARM.

Please select action "Resign revision" (or something similar, I can't remember the actual text).

Jun 23 2021, 9:17 AM · Restricted Project
rengolin added a comment to D104759: Indicate ABI in ARM ELF header flags.

@peter.smith, thank you for the analysis! I'm consequently backing out of this patch.

Jun 23 2021, 9:16 AM · Restricted Project
rengolin added reviewers for D104759: Indicate ABI in ARM ELF header flags: kristof.beyls, peter.smith, rovka.

Adding folks working on Arm currently.

Jun 23 2021, 6:43 AM · Restricted Project

Jun 22 2021

rengolin added a comment to D104264: Fix atomic loads and stores of 64-bit values in non-default address spaces on ARM.
Jun 22 2021, 9:34 AM · Restricted Project
rengolin added a comment to D104264: Fix atomic loads and stores of 64-bit values in non-default address spaces on ARM.

Can someone close this review as "Won't fix"?

Jun 22 2021, 9:24 AM · Restricted Project
rengolin added a comment to D104264: Fix atomic loads and stores of 64-bit values in non-default address spaces on ARM.

Back ends are expected to crash when given invalid IR.

Jun 22 2021, 9:18 AM · Restricted Project
rengolin added a comment to D104264: Fix atomic loads and stores of 64-bit values in non-default address spaces on ARM.

The Morello (AArch64 + CHERI) implementation uses AS 200 for capabilities and does different instruction selection for loads and stores (including atomics) depending on the address space.

The back end should never see IR that contains address spaces that don't exist for that back end. I'd expect that code wanting to use AS1 to indicate GC pointers should rewrite them to AS0 immediately before codegen.

Jun 22 2021, 5:40 AM · Restricted Project

Jun 16 2021

rengolin added a comment to D102861: Suppress FP_CONTRACT due to planned command line changes.

I can see there's aarch64 and x86 but what about other arch?

Jun 16 2021, 3:28 AM

Jun 15 2021

rengolin added a comment to D102861: Suppress FP_CONTRACT due to planned command line changes.

I plan to try pushing again when I have a block of time to babysit the bots. Is the place I should watch on Green Dragon? e.g. https://green.lab.llvm.org/green/job/test-suite-verify-machineinstrs-x86_64h-O3/
I can see there's aarch64 and x86 but what about other arch?

Jun 15 2021, 3:42 PM
rengolin added a comment to D102861: Suppress FP_CONTRACT due to planned command line changes.

BTW the reason that 2mm 3mm and gemm didn't build is because I was using -DTEST_SUITE_BENCHMARKING_ONLY=true on the cmake invocation, so those 3 test directories were not included.

Jun 15 2021, 1:44 PM
rengolin added a comment to D104264: Fix atomic loads and stores of 64-bit values in non-default address spaces on ARM.

They are target defined. You're only doing this for this one target, so you can make them mean whatever you want them to mean

Jun 15 2021, 11:58 AM · Restricted Project
rengolin added a comment to D104264: Fix atomic loads and stores of 64-bit values in non-default address spaces on ARM.

Those are great questions! :)

Jun 15 2021, 10:30 AM · Restricted Project
rengolin added a comment to D102861: Suppress FP_CONTRACT due to planned command line changes.

I can resurrect some Armv7/v8 machines at home and try

Jun 15 2021, 9:37 AM
rengolin added a comment to D104264: Fix atomic loads and stores of 64-bit values in non-default address spaces on ARM.

If I understand well, you rather opt for choice (2): Atomic loads and stores are legal only on the default address space.

Jun 15 2021, 9:33 AM · Restricted Project
rengolin added a comment to D104264: Fix atomic loads and stores of 64-bit values in non-default address spaces on ARM.

The use of a different address space in my case is used to distinguish "pointers to native memory" from "pointers to managed objects" in a Java virtual machine.

Jun 15 2021, 7:34 AM · Restricted Project
rengolin added a reviewer for D104264: Fix atomic loads and stores of 64-bit values in non-default address spaces on ARM: theraven.

Hi Koutheir,

Jun 15 2021, 12:44 AM · Restricted Project

Jun 14 2021

rengolin updated subscribers of D102861: Suppress FP_CONTRACT due to planned command line changes.

This patch affects multiple architectures. Is there a way to submit this patch for testing before pushing to the trunk?

Jun 14 2021, 12:49 PM

Jun 12 2021

rengolin added a comment to D103906: Do not generate calls to the 128-bit function __multi3() on 32-bit ARM..

The build builders/53/builds/3020 fails due a time out. I don't think this is an actual failure. Maybe someone could increase the time out value for certain long-running tests.

Jun 12 2021, 1:18 PM · Restricted Project

Jun 11 2021

rengolin added a comment to D103906: Do not generate calls to the 128-bit function __multi3() on 32-bit ARM..

I'm having trouble building and testing, if anyone else could commit this for the time being, I'd appreciate it. Thanks!

Jun 11 2021, 3:47 AM · Restricted Project
rengolin committed rG789708617d20: Do not generate calls to the 128-bit function __multi3() on 32-bit ARM (authored by koutheir).
Do not generate calls to the 128-bit function __multi3() on 32-bit ARM
Jun 11 2021, 3:46 AM

Jun 10 2021

rengolin added a comment to D104055: Override ffp-contract to fast due to command line changes planned in D74436.

Probably cleaner to revert, let the bots calm down, then reapply the right patch.

Jun 10 2021, 3:16 PM · Restricted Project
rengolin added a comment to D103906: Do not generate calls to the 128-bit function __multi3() on 32-bit ARM..

I'm having trouble building and testing, if anyone else could commit this for the time being, I'd appreciate it. Thanks!

Jun 10 2021, 4:01 AM · Restricted Project

Jun 9 2021

rengolin added a comment to D103906: Do not generate calls to the 128-bit function __multi3() on 32-bit ARM..

@rengolin I tested the ARM tests that I thought would be affected. This appears to be affecting other tests.

Jun 9 2021, 1:04 PM · Restricted Project
rengolin added a comment to D103906: Do not generate calls to the 128-bit function __multi3() on 32-bit ARM..
Jun 9 2021, 11:57 AM · Restricted Project
rengolin added a comment to D103906: Do not generate calls to the 128-bit function __multi3() on 32-bit ARM..

Done. I have run tests locally and they were fine, but please look out for buildbot emails to make sure it didn't break anything we didn't test. Thanks!

Jun 9 2021, 8:23 AM · Restricted Project
rengolin committed rG64e9aa33020d: Do not generate calls to the 128-bit function __multi3() on 32-bit ARM (authored by koutheir).
Do not generate calls to the 128-bit function __multi3() on 32-bit ARM
Jun 9 2021, 8:22 AM
rengolin closed D103906: Do not generate calls to the 128-bit function __multi3() on 32-bit ARM..
Jun 9 2021, 8:21 AM · Restricted Project
rengolin accepted D103906: Do not generate calls to the 128-bit function __multi3() on 32-bit ARM..

Right, I think it's a bit clearer now.

Jun 9 2021, 2:25 AM · Restricted Project

Jun 8 2021

rengolin updated subscribers of D103906: Do not generate calls to the 128-bit function __multi3() on 32-bit ARM..

Right, ok, this is true for compiler-rt, but not necessarily other runtime libraries.

Jun 8 2021, 11:02 AM · Restricted Project
rengolin added a comment to D103906: Do not generate calls to the 128-bit function __multi3() on 32-bit ARM..

Doesn't this depend on which runtime library you're using? No Arm32 library implement that?

Jun 8 2021, 9:35 AM · Restricted Project

May 29 2021

rengolin accepted D103360: Add a toplevel .mailmap file.

Fair enough

May 29 2021, 11:04 AM · Restricted Project
rengolin added a comment to D103360: Add a toplevel .mailmap file.

Can you add your aliases right off the bat? I have to say I was confused by the git manual, maybe I'm not the only one. Would be nice to have an example to follow...

May 29 2021, 10:31 AM · Restricted Project

May 28 2021

rengolin updated subscribers of D53927: [AArch64] Enable libm vectorized functions via SLEEF.
  1. If I somehow obtain access to an AArch64 box to re-do this patch (I'm fuzzy on that at the moment), and I spend my own free time doing the work, I would like to know ahead of time that it will not degenerate again into what went on here from 2018 through 2019.
May 28 2021, 7:25 AM · Restricted Project

May 27 2021

rengolin added a comment to D102861: Suppress FP_CONTRACT due to planned command line changes.

Perhaps it would be easier to just remove the part of the code that runs twice? Probably also need to remove the output files and maybe some CMake magic that compares them?

May 27 2021, 3:20 PM
rengolin accepted D102861: Suppress FP_CONTRACT due to planned command line changes.

I'm ok with that, for now. Medium term, it would be best to just remove the second run, but I don't mind it not being in this patch.

May 27 2021, 10:31 AM
rengolin added a comment to D102861: Suppress FP_CONTRACT due to planned command line changes.

I agree with you Renato: polybench should be run once.
I would recommend to run polybench without checking for correctness issues.
The original purpose of polybench is to be a performance benchmark.
The original code of polybench did not contain correctness checks.
The correctness checks have been added by LLVM developers, and checking for correctness is a hard problem for FP.

May 27 2021, 2:12 AM

May 26 2021

rengolin added a comment to D102861: Suppress FP_CONTRACT due to planned command line changes.

https://reviews.llvm.org/D74436 is a very important change: thanks @mibintc for working on this!
https://reviews.llvm.org/D25277 and other changes to LLVM test-suite was motivated by a similar change in Clang to make the default behavior similar to what GCC does for fp contraction.

May 26 2021, 11:14 AM
rengolin added a comment to D102861: Suppress FP_CONTRACT due to planned command line changes.

Since the pragma overrides the command line option, contraction will always be suppressed in the affected region of source code, regardless if ffp-contract=on on the command line.

May 26 2021, 8:45 AM

May 25 2021

rengolin requested changes to D102861: Suppress FP_CONTRACT due to planned command line changes.

I believe most or all were added in https://reviews.llvm.org/D25346 ; I can create another revision of this patch which suppresses for the entire test using the command line option.

May 25 2021, 3:17 AM
rengolin added a reviewer for D102861: Suppress FP_CONTRACT due to planned command line changes: kristof.beyls.

This is fine long-term for applications, but some groups monitor these benchmark numbers. I'm adding @kristof.beyls but I'm sure there are more.

May 25 2021, 3:05 AM

May 21 2021

rengolin added a comment to D53927: [AArch64] Enable libm vectorized functions via SLEEF.

This was a long time ago, I completely forgot about this.

May 21 2021, 3:56 AM · Restricted Project

Apr 30 2021

rengolin added a reviewer for D101410: [M68k][test] Initial migration of MC tests: zixuan-wu.

Adding @zixuan-wu as a reviewer, who's also adding a new target and could help with the recently acquired expertise. :)

Apr 30 2021, 6:14 AM · Restricted Project

Apr 23 2021

rengolin added a comment to D101027: [CSKY] Add more basic instructions including some privilege instructions.

Because there is other patch between 7 and 9, which I will make a revision later.

Apr 23 2021, 1:28 AM · Restricted Project

Apr 22 2021

rengolin added a comment to D101027: [CSKY] Add more basic instructions including some privilege instructions.

The old 7 is now 9 and there's no 8? This is confusing.

Apr 22 2021, 6:52 AM · Restricted Project

Apr 21 2021

rengolin accepted D100372: [Clang][ARM] Define __VFP_FP__ macro unconditionally.

It's a weird flag, for sure, but if that's the semantics of it, than this change LGTM. Thanks!

Apr 21 2021, 2:48 AM · Restricted Project