rengolin (Renato Golin)
Toolchain Engineer

Projects

User does not belong to any projects.

User Details

User Since
Oct 19 2012, 12:57 AM (260 w, 4 d)

Recent Activity

Mon, Oct 16

rengolin added a comment to D38676: [LV] Model masking in VPlan, introducing VPInstructions.

FYI: http://lists.llvm.org/pipermail/llvm-dev/2017-October/118241.html

Mon, Oct 16, 4:59 AM

Sun, Oct 15

rengolin added a comment to D38676: [LV] Model masking in VPlan, introducing VPInstructions.
In D38676#897413, @gilr wrote:

To avoid introducing dead code into LV we're adding the new VPlan constructs along with a single, well-bounded use case.

Sun, Oct 15, 8:40 AM
rengolin added a comment to D38676: [LV] Model masking in VPlan, introducing VPInstructions.

A minor nit but LGTM. I don't have an aversion to "dead code" if it's going to be used in the near future, so perhaps to make the patch smaller split it into an initial patch to add VPlanValue.h and then the VPlanBuilder.h as well as the changes to the documentation. Those pieces seem uncontroversial to me, before moving onto the vectorizer. @mkuper does this make sense?

Sun, Oct 15, 8:36 AM

Thu, Oct 12

rengolin added a comment to D38676: [LV] Model masking in VPlan, introducing VPInstructions.

Hi Ayal/Gil,

Thu, Oct 12, 8:13 AM

Fri, Oct 6

rengolin added a reviewer for D38621: [AArch64][GlobalISel] Make G_PHI of p0 types legal: rovka.
Fri, Oct 6, 6:08 AM

Thu, Oct 5

rengolin added a comment to D38588: Clear LastMappingSymbols and LastEMS(Info) when resetting the ARM(AArch64)ELFStreamer.

What do you mean by "and in reset"? (This is reset function). In case there's confusion, the two part of the patch are on two targets and they are unrelated though should be fixing the same issue.

Thu, Oct 5, 9:15 AM
rengolin added a comment to D38588: Clear LastMappingSymbols and LastEMS(Info) when resetting the ARM(AArch64)ELFStreamer.

I don't see anything terribly wrong in cleaning up between object emissions, but I'll let @t.p.northover and @peter.smith make sure this can't be done in a better way (like fixing the original patch), in which case, we might want at least an assert here.

Thu, Oct 5, 9:02 AM

Wed, Oct 4

rengolin added a comment to D38479: Make -mgeneral-regs-only more like GCC's.
  1. We don't correctly ignore inline asm clobbers for registers which aren't allocatable (https://bugs.llvm.org/show_bug.cgi?id=30792)
Wed, Oct 4, 5:41 AM
rengolin accepted D38339: [LV] Fix PR34711 - handle widening of instruction ranges in the presence of sinking casts.

LGTM. Thanks!

Wed, Oct 4, 1:45 AM
rengolin updated subscribers of D38143: Dynamic stack alignment for Thumb1.
Wed, Oct 4, 1:37 AM
rengolin added a comment to D38175: [ARM] Make sure assembler rejects PC as an operand for VMOV.F16.

All in all, this looks like a bigger job than I initially hoped for. So it might take a little longer to investigate and produce a patch...

Wed, Oct 4, 1:09 AM

Tue, Oct 3

rengolin committed rL314796: [release_50] Merging r313916.
[release_50] Merging r313916
Tue, Oct 3, 6:15 AM

Wed, Sep 27

rengolin accepted D37788: [ARM] builtins: Replace abort by assert in clear_cache..

Let's see how this goes... :)

Wed, Sep 27, 7:47 AM
rengolin added a comment to D37788: [ARM] builtins: Replace abort by assert in clear_cache..

I agree with Peter analysis and accept Saleem's solution as a compromise.

Wed, Sep 27, 12:26 AM

Fri, Sep 22

rengolin added a comment to D38175: [ARM] Make sure assembler rejects PC as an operand for VMOV.F16.

All similar VMOVs seem to have the same restriction, maybe we should update the whole lot at the same time?

Fri, Sep 22, 8:08 AM
rengolin added inline comments to D38143: Dynamic stack alignment for Thumb1.
Fri, Sep 22, 8:01 AM

Wed, Sep 20

rengolin added a comment to D38050: [ARM] Use correct calling convention for libm..

Do you know what would be cool? A gcc-compatibility checker test... Something that would have the most problematic cases in C and asm files and would be compiled with multiple versions of gcc and clang triples, then compared.

Wed, Sep 20, 3:37 PM

Tue, Sep 19

rengolin added a reviewer for D38052: [AArch64] Implement R_AARCH64_ LD_PREL_LO19 : peter.smith.
Tue, Sep 19, 2:06 PM
rengolin added a reviewer for D38050: [ARM] Use correct calling convention for libm.: peter.smith.
Tue, Sep 19, 2:05 PM

Mon, Sep 18

rengolin added a comment to D31951: TableGen support for parameterized register class information.
In D31951#873924, @rnk wrote:

+1, a 4x runtime regression in tablegen isn't acceptable, and is something that definitely should've been caught before hand.

Mon, Sep 18, 10:42 AM
rengolin added a comment to D37788: [ARM] builtins: Replace abort by assert in clear_cache..

I don't think the above argument is strong enough to go against the consensus opinion though.

Mon, Sep 18, 8:41 AM
rengolin added a comment to D37788: [ARM] builtins: Replace abort by assert in clear_cache..

Some more info on this. At the time Android did its implementation, the Linux kernel used[1] to call do_cache_op, ignoring the return result and then returning zero. Now [2], it returns whatever do_cache_op returns, which ends up being a very long sequence of uses and definitions, boiling down to SVC.

Mon, Sep 18, 7:39 AM
rengolin added a comment to D37738: [SLPVectorizer] Generalize vectorizeStores to support loads as well NFC. .

tests? benchmarks results?

Mon, Sep 18, 7:12 AM
rengolin accepted D37974: [AArch64] Add V8_2aOps feature to Cortex-A55 and 75.
Mon, Sep 18, 7:10 AM
rengolin accepted D37968: [ARM] Fix for indexed dot product instruction descriptions.

It's in the pseudo-code:

Mon, Sep 18, 7:07 AM
rengolin added a comment to D37737: [SLPVectorizer] Merge subsequent gather loads..

Hi Florian,

Mon, Sep 18, 6:59 AM
rengolin added a comment to D37968: [ARM] Fix for indexed dot product instruction descriptions.

Ignore me, that was VSDOT... :)

Mon, Sep 18, 6:07 AM
rengolin added a comment to D37968: [ARM] Fix for indexed dot product instruction descriptions.

Interesting, the Aarch32 PDF doesn't have SDOT/UDOT, only the AArch64 one...

Mon, Sep 18, 5:54 AM
rengolin added a comment to D37968: [ARM] Fix for indexed dot product instruction descriptions.

The last ARMv8.2 manual I could find is from 31 March, but it says UDOT/SDOT will be documented later. Do you have an update on that?

Mon, Sep 18, 4:13 AM
rengolin accepted D37967: [LoopVectorizer] Add more testcases for PR33804..

LGTM. Thanks!

Mon, Sep 18, 3:48 AM

Sep 13 2017

rengolin added a comment to D37788: [ARM] builtins: Replace abort by assert in clear_cache..

Sorry, pressed ctrl+enter by accident.

Sep 13 2017, 12:06 PM
rengolin added a comment to D37788: [ARM] builtins: Replace abort by assert in clear_cache..
  • The prototype for the builtin is void builtin_clear_cache (char *begin, char *end) so there isn't a way of reporting failure from that function to the caller
  • The system call can fail on Linux when StartAddress > EndAddress which the user can check themselves before calling, or if the access_ok check fails (see comment later) or if a data abort exception occurs when doing one of the CP15 cache manipulation instructions. According to the linux kernel code, this happens when the virtual address isn't mapped. So it looks like a program has given builtin_clear_cache an invalid memory range.
Sep 13 2017, 12:01 PM
rengolin added a comment to D37788: [ARM] builtins: Replace abort by assert in clear_cache..

Digging it a bit, commit r203674 (reviewed by dsanders/bastien) introduced this part for Android ARM/MIPS.

Sep 13 2017, 9:54 AM
rengolin added a reviewer for D37788: [ARM] builtins: Replace abort by assert in clear_cache.: joerg.
Sep 13 2017, 4:21 AM
rengolin requested changes to D37788: [ARM] builtins: Replace abort by assert in clear_cache..

Do you have any evidence that this behaviour was reached by design and not accident?

Sep 13 2017, 4:21 AM
rengolin updated subscribers of D37798: Teach TargetInstrInfo::getInlineAsmLength to parse .space directives with integer arguments.
Sep 13 2017, 3:52 AM

Sep 8 2017

rengolin accepted D37619: [LV] Fix PR34523 - avoid generating redundant selects.

I can't see anything immediately wrong with this patch, it looks trivial and correct. Moreover, if this was done because of a know problem (unlikely, given the commit it came from), we have the assert to help.

Sep 8 2017, 4:51 AM

Sep 6 2017

rengolin accepted D37459: [buildbot] Remove clang-cmake-aarch64-42vma builder.

We have dropped 39-bit testing a while ago and now that all AArch64 Linux kernels are moving to 48-bits, we don't need to support every possible variation.

Sep 6 2017, 3:25 AM

Sep 4 2017

rengolin committed rL312482: Revert "[test-suite] Adding the CLAMR mini-app".
Revert "[test-suite] Adding the CLAMR mini-app"
Sep 4 2017, 4:29 AM

Sep 1 2017

rengolin added a comment to D37377: [ARM] Add 2-operand assembly aliases for Thumb1 ADD/SUB.

There might be my fault because I put my comment with the action 'Accept revision'. My understanding of the use of 'Accept revision' is that I have reviewed a patch and I accept it. Further steps, especially when to commit, should be done according to the LLVM developer policy. Maybe my understanding of this is wrong.

Sep 1 2017, 6:46 AM
rengolin added a comment to D37377: [ARM] Add 2-operand assembly aliases for Thumb1 ADD/SUB.

As I said in D37374, please refrain from approving your own patches within minutes of posting. The community must be part of the process, and if people are not reviewing your patches as fast as you want, then there's either something wrong with the community (and we need to fix), or with your expectations.

Sep 1 2017, 3:56 AM
rengolin added a comment to D37374: [PATCH][ARM] Enable the use of SVC anywhere in an IT block.

Folks, please refrain from approving your own patches minutes after posting. This is not how open source is done. If you need review, let people review your patches. Peter's comment is a clear example on why this is important.

Sep 1 2017, 3:51 AM

Aug 31 2017

rengolin added a comment to D35498: [LoopVectorizer] Use two step casting for float to pointer types..

Sorry, slight overlap. Please follow Ayal's comments before committing.

Aug 31 2017, 2:10 AM
rengolin accepted D35498: [LoopVectorizer] Use two step casting for float to pointer types..

Sorry for the delay. It looks good to me now, thanks!

Aug 31 2017, 2:10 AM

Aug 27 2017

rengolin added a comment to D37052: Add default address space for functions to the data layout (1/4).

Has there been an RFC on this?

Aug 27 2017, 10:50 AM

Aug 23 2017

rengolin committed rL311578: [ARM] more release notes updates for 5.0.
[ARM] more release notes updates for 5.0
Aug 23 2017, 10:06 AM
rengolin added inline comments to D37030: Fix ARMv4 support.
Aug 23 2017, 3:30 AM
rengolin accepted D37040: LowerAtomic: Don't skip optnone functions; atomic still need lowering (PR34020).
Aug 23 2017, 1:08 AM

Aug 22 2017

rengolin closed D35821: Have ARM call setBooleanContents(ZeroOrOneBooleanContent)..
Aug 22 2017, 4:04 AM
rengolin closed D36705: Avoid creating duplicate ANDs in SelectionDAG..
Aug 22 2017, 4:04 AM
rengolin committed rL311446: [ARM] Call setBooleanContents(ZeroOrOneBooleanContent).
[ARM] Call setBooleanContents(ZeroOrOneBooleanContent)
Aug 22 2017, 4:03 AM
rengolin committed rL311447: [ARM] Avoid creating duplicate ANDs in SelectionDAG.
[ARM] Avoid creating duplicate ANDs in SelectionDAG
Aug 22 2017, 4:03 AM

Aug 21 2017

rengolin added a comment to D36830: Use report_fatal_error for unsupported calling conventions.

Agreed.

Aug 21 2017, 2:03 PM
rengolin added inline comments to D35498: [LoopVectorizer] Use two step casting for float to pointer types..
Aug 21 2017, 8:38 AM

Aug 19 2017

rengolin accepted D36899: [ARM] Check the right order for halves of VZIP/VUZP if both parts are used.

Hi Martin,

Aug 19 2017, 11:30 AM

Aug 18 2017

rengolin added a comment to D36731: [ARM][AArch64] Cortex-A75 and Cortex-A55 tests.

I'm happy with the patch, but I'll let @SjoerdMeijer approve.

Aug 18 2017, 6:50 AM
rengolin added a comment to D36731: [ARM][AArch64] Cortex-A75 and Cortex-A55 tests.

This change seems to have nothing to do with adding core support but exposing features from CPU names.

Aug 18 2017, 4:11 AM
rengolin committed rL311153: [Triple] Define OS Check for Haiku.
[Triple] Define OS Check for Haiku
Aug 18 2017, 3:38 AM
rengolin closed D36814: [Triple] Define OS Check for Haiku.

This patch is needed in-tree for upstreaming my Swift port for Haiku, which is part of my Google Summer of Code project.

The swift-llvm fork regularly pulls changes from LLVM master into the upstream-with-swift branch, so adding this patch here should make sense.

Aug 18 2017, 3:37 AM

Aug 17 2017

rengolin accepted D36814: [Triple] Define OS Check for Haiku.

Curiosity: Do you have a use for this in-tree or just off-tree?

Aug 17 2017, 10:57 AM

Aug 15 2017

rengolin accepted D36732: [ARM, Asm] Harden GNU LDRD/STRD aliases against invalid inputs.

Ok, LGTM. Thanks!

Aug 15 2017, 5:04 AM
rengolin added inline comments to D36732: [ARM, Asm] Harden GNU LDRD/STRD aliases against invalid inputs.
Aug 15 2017, 4:21 AM
rengolin added a reviewer for D36705: Avoid creating duplicate ANDs in SelectionDAG.: john.brawn.
Aug 15 2017, 2:23 AM

Aug 14 2017

rengolin added a reviewer for D31878: [Asm] Add debug tracing in table-generated assembly matcher: echristo.

Adding Eric, for consistency.

Aug 14 2017, 11:58 AM
rengolin accepted D31537: [ARM, Asm] Use correct source location for register tokens.
Aug 14 2017, 11:57 AM
rengolin accepted D36689: [ARM, Asm] Change grammar of immediate operand diagnostics.

NFC. LGTM. Thanks!

Aug 14 2017, 11:56 AM
rengolin accepted D36692: [ARM, Asm] Add diagnostics for general-purpose register operands.

Other than the one comment, LGTM. Thanks!

Aug 14 2017, 11:54 AM
rengolin accepted D36693: [ARM, Asm] Add diagnostics for floating-point register operands.
Aug 14 2017, 11:52 AM
rengolin added a comment to D35821: Have ARM call setBooleanContents(ZeroOrOneBooleanContent)..

Submit as a different revision and we can discuss the merits there.

Aug 14 2017, 11:46 AM
rengolin accepted D35821: Have ARM call setBooleanContents(ZeroOrOneBooleanContent)..

LGTM, thanks!

Aug 14 2017, 11:16 AM
rengolin added a comment to D27620: [Assembler] Report multiple near misses for invalid instructions.

Hi Oliver,

Aug 14 2017, 9:05 AM

Aug 12 2017

rengolin accepted D34682: [Triple] Add isThumb and isARM functions..

Yup. LGTM. Thanks!

Aug 12 2017, 8:24 AM
rengolin added a comment to D35821: Have ARM call setBooleanContents(ZeroOrOneBooleanContent)..

There are two patches here, please split them.

Aug 12 2017, 6:29 AM

Aug 11 2017

rengolin added a comment to D31220: [builtins][ARM] Select correct code fragments when compiling for Thumb1/Thum2/ARM ISA.

ping @compnerd

Aug 11 2017, 3:05 PM
rengolin added inline comments to D34682: [Triple] Add isThumb and isARM functions..
Aug 11 2017, 6:06 AM
rengolin accepted D36609: [AArch64] Remove dotprod from base extension list.
Aug 11 2017, 5:51 AM

Aug 3 2017

rengolin accepted D35569: [ARM] Remove FeatureNoARM implies ModeThumb..

Sorry, long holidays... :)

Aug 3 2017, 11:34 PM
rengolin accepted D35826: [Driver] Error if ARM mode was selected explicitly for M-profile CPUs..

This makes sense to me. LGTM. Thanks!

Aug 3 2017, 11:30 PM

Jul 28 2017

rengolin accepted D35568: [AArch64] Use 8 bytes as preferred function alignment on Cortex-A53..

LGTM too. Thanks!

Jul 28 2017, 3:23 PM
rengolin added inline comments to D34682: [Triple] Add isThumb and isARM functions..
Jul 28 2017, 3:22 PM

Jul 25 2017

rengolin added a reviewer for D35826: [Driver] Error if ARM mode was selected explicitly for M-profile CPUs.: charles.baylis.
Jul 25 2017, 7:20 AM

Jul 24 2017

rengolin added a comment to D35568: [AArch64] Use 8 bytes as preferred function alignment on Cortex-A53..

Using 8 byte alignment gives a 0.25% speedup on execution time (was 0.23% with 16 bytes), a 0.82% improvement
in benchmark scores (was 0.93% with 16 bytes) and a 0.20% increase in binary size (was 0.55%). So for the score related benchmarks, the 8 byte alignment makes things worse quite a bit, but the impact on size is much smaller. Should we use 8 byte alignment, to keep the binary size down?

Jul 24 2017, 5:31 AM

Jul 23 2017

rengolin accepted D35620: [AArch64] Add test for function alignment for a optsize function (NFC). .
Jul 23 2017, 11:06 AM

Jul 21 2017

rengolin added a comment to D35568: [AArch64] Use 8 bytes as preferred function alignment on Cortex-A53..

So, it seems it was sphinx, but that was loop alignment, 4 bytes on A53, 8 bytes on A57, to do with the fetch alignment. Maybe this is a related issue. Why 16, though?

Jul 21 2017, 4:32 PM
rengolin added a comment to D35568: [AArch64] Use 8 bytes as preferred function alignment on Cortex-A53..

We found similar results on spec2k6 for aarch64 that we attributed to function alignment. Have you tried that? I need to dig the one culprit...

Jul 21 2017, 4:24 PM

Jul 20 2017

rengolin added reviewers for D35696: Fix compiler-rt build with aarch64 using -march=armv8-a: zatrazz, peter.smith.
Jul 20 2017, 1:55 PM
rengolin added reviewers for D35697: Fix libcxx build with glibc 2.26+: zatrazz, peter.smith.
Jul 20 2017, 1:55 PM
rengolin added a comment to D35569: [ARM] Remove FeatureNoARM implies ModeThumb..

With FeatureARM, the generic CPU would have to have this feature, but when the features of the architecture (v6m) are applied, FeatureARM cannot be removed, so the behavior will be different to the current one (Architecture features are applied after CPU features I think). We could rename FeatureARM to FeatureThumbOnly, which is a bit clearer in my opinion.

Jul 20 2017, 9:15 AM
rengolin accepted D33410: Correct test for ARM targets.

Apologies, you had already sent all I needed. :)

Jul 20 2017, 4:12 AM

Jul 19 2017

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

Hi Ayal,

Jul 19 2017, 6:37 AM
rengolin added inline comments to D35498: [LoopVectorizer] Use two step casting for float to pointer types..
Jul 19 2017, 3:48 AM

Jul 18 2017

rengolin added a comment to D35498: [LoopVectorizer] Use two step casting for float to pointer types..
typedef struct CvNode1D
{
    float val;
    struct CvNode1D *next;
}
CvNode1D;

And we're trying to vectorize code that loads these structs. Since the pointer and the float have the same width, we can load four of the structs as, e.g. 2 * <4 x float>, and then use shuffles to get a vector of 4 floats and a vectors of 4 pointers.

Jul 18 2017, 9:13 AM

Jul 17 2017

rengolin added a comment to D35498: [LoopVectorizer] Use two step casting for float to pointer types..

Setting aside from the fact that no one should ever be casting floating point to pointers, perhaps we shouldn't be even trying to do this transformation here, as I'm not sure this could have a guaranteed semantics in this case, and probably better to just bail the vectorisation?

Jul 17 2017, 12:50 PM

Jul 14 2017

rengolin committed rL308006: [RelTest] Diana is doing both releases now.
[RelTest] Diana is doing both releases now
Jul 14 2017, 1:34 AM

Jul 13 2017

rengolin added a comment to D35307: [AArch64] Initial SVE register definitions.

This seems a very raw change, without any further description, comments or proper usage, other than a few changes on random places.

I don't agree. The additions are in line with the level of documentation in the rest of the file.

Jul 13 2017, 10:38 AM
rengolin accepted D35076: [AArch64] Add an SVE target feature to the backend and TargetParser.

The Clang counterpart was approved, this looks good, too. Thanks!

Jul 13 2017, 3:55 AM
rengolin accepted D35118: [AArch64] Add support for handling the +sve target feature.

Thanks. LGTM.

Jul 13 2017, 3:55 AM · Restricted Project
rengolin added a comment to D35118: [AArch64] Add support for handling the +sve target feature.

@jmolloy Can you check this change, please?

I'm not really removing FPUMode, I'm just converting an enum to a bit field. FPUMode, i.e. no NEON, after this change is now represented by simply having all bits be 0. See the equivalent implementation for ARM which does the same thing.

Jul 13 2017, 2:41 AM · Restricted Project

Jul 12 2017

rengolin added a reviewer for D35118: [AArch64] Add support for handling the +sve target feature: jmolloy.

@jmolloy Can you check this change, please?

Jul 12 2017, 9:17 AM · Restricted Project
rengolin added a comment to D35307: [AArch64] Initial SVE register definitions.

Hi Amara,

Jul 12 2017, 9:10 AM