Page MenuHomePhabricator

hgreving (Hendrik Greving)
User

Projects

User does not belong to any projects.

User Details

User Since
Jan 31 2020, 1:07 PM (38 w, 1 d)

Recent Activity

Fri, Oct 16

hgreving added a comment to rG0345d88de654: [NFC][ScheduleDAG] Remove unused EntrySU SUnit.

I see. Yes this seems to be just waiting for somebody else trying to remove it.

Fri, Oct 16, 9:52 AM
hgreving added a comment to D87867: [NFC][ScheduleDAG] Remove unused EntrySU SUnit.

Our backend is using EntrySU the same way as described by another poster above: to add top edges to adjust for clearances to the basic block entry. When removing this, we need to add our own EntrySU, but this becomes tricky because the scheduler will try to schedule it and release the node. I think it does make sense to keep EntrySU in-tree. Thanks for that.

Fri, Oct 16, 9:47 AM · Restricted Project
hgreving added a comment to rG0345d88de654: [NFC][ScheduleDAG] Remove unused EntrySU SUnit.

Is this really necessary to remove? For example, our backend is using EntrySU edges for top edges from the basic block entry, in order to account for clearances. It seems to make sense to have a node like this.

Fri, Oct 16, 9:31 AM

Aug 3 2020

hgreving committed rG509f5c4ec2db: [MC] Fix memory leak when allocating MCInst with bump allocator (authored by hgreving).
[MC] Fix memory leak when allocating MCInst with bump allocator
Aug 3 2020, 4:08 PM

Jul 31 2020

hgreving added a comment to D84896: [MC] Fix memory leak when allocating MCInst with bump allocator.

Tending to push this rel. small fix w/o an explicit Hexagon sign-off. Objections?

Jul 31 2020, 5:04 PM · Restricted Project

Jul 30 2020

hgreving updated the summary of D84896: [MC] Fix memory leak when allocating MCInst with bump allocator.
Jul 30 2020, 10:26 AM · Restricted Project
hgreving added a comment to D84896: [MC] Fix memory leak when allocating MCInst with bump allocator.

Done.

Jul 30 2020, 10:25 AM · Restricted Project
hgreving updated the diff for D84896: [MC] Fix memory leak when allocating MCInst with bump allocator.
Jul 30 2020, 10:24 AM · Restricted Project

Jul 29 2020

hgreving updated the summary of D84896: [MC] Fix memory leak when allocating MCInst with bump allocator.
Jul 29 2020, 5:52 PM · Restricted Project
hgreving added a comment to D84896: [MC] Fix memory leak when allocating MCInst with bump allocator.

I have added some people w/o knowing them based on contributions to Hexagon and MC. Please accept or reassign the review if possible.

Jul 29 2020, 5:13 PM · Restricted Project
hgreving updated the summary of D84896: [MC] Fix memory leak when allocating MCInst with bump allocator.
Jul 29 2020, 5:11 PM · Restricted Project
hgreving updated the summary of D84896: [MC] Fix memory leak when allocating MCInst with bump allocator.
Jul 29 2020, 5:01 PM · Restricted Project
hgreving requested review of D84896: [MC] Fix memory leak when allocating MCInst with bump allocator.
Jul 29 2020, 4:53 PM · Restricted Project

Jul 6 2020

hgreving added a comment to D82673: [ModuloSchedule] Make PeelingModuloScheduleExpander inheritable..

In short - the virtuality is not the issue, the inheritability is. The class can be non-virtual, no problem for us. I would like to reuse existing code in the upstream pass that is currently "private". Hence, the "protected" part is important. It also makes sense generally, because a software pipeliner on a specific subtarget may follow different expansion strategies. If you would like to revert the virtuality, no problem at all, if you keep the protected inheritance. I think we should design a better defined interface for this class in the long run. Only one target upstream and us downstream are using it AFAIK, but as we are supporting more targets, we may come up with something better. SG? Can pull in Hexagon people, yes

Ah, fair enough. Thanks for explaining!

I've removed the virtual dtor and virtuality from expand in 7a99aab8692c58558b62e9a66120886b8a70fab8

Jul 6 2020, 6:42 PM · Restricted Project

Jul 2 2020

hgreving added a comment to D82673: [ModuloSchedule] Make PeelingModuloScheduleExpander inheritable..

In short - the virtuality is not the issue, the inheritability is. The class can be non-virtual, no problem for us. I would like to reuse existing code in the upstream pass that is currently "private". Hence, the "protected" part is important. It also makes sense generally, because a software pipeliner on a specific subtarget may follow different expansion strategies. If you would like to revert the virtuality, no problem at all, if you keep the protected inheritance. I think we should design a better defined interface for this class in the long run. Only one target upstream and us downstream are using it AFAIK, but as we are supporting more targets, we may come up with something better. SG? Can pull in Hexagon people, yes

Jul 2 2020, 8:31 PM · Restricted Project

Jul 1 2020

hgreving added a comment to D82673: [ModuloSchedule] Make PeelingModuloScheduleExpander inheritable..

Adding an extension point with no testing inside LLVM seems a bit problematic - is there no existing target that could use this? Or a reasonable scale of unit test that could exercise this extension point without a full-blown target?

(also this produced a warning about the fact that this type doesn't have a virtual dtor - was fixed by @jfb in c7586444ca787c3845ac4ad0bd603709f2abbb0f )

Jul 1 2020, 10:48 AM · Restricted Project

Jun 30 2020

hgreving committed rG50ac7ce94f34: [ModuloSchedule] Make PeelingModuloScheduleExpander inheritable. (authored by hgreving).
[ModuloSchedule] Make PeelingModuloScheduleExpander inheritable.
Jun 30 2020, 4:19 PM
hgreving closed D82673: [ModuloSchedule] Make PeelingModuloScheduleExpander inheritable..
Jun 30 2020, 4:19 PM · Restricted Project

Jun 26 2020

hgreving updated the summary of D82673: [ModuloSchedule] Make PeelingModuloScheduleExpander inheritable..
Jun 26 2020, 12:04 PM · Restricted Project
hgreving created D82673: [ModuloSchedule] Make PeelingModuloScheduleExpander inheritable..
Jun 26 2020, 12:02 PM · Restricted Project

Jun 8 2020

hgreving committed rGf3d8a9397003: [ModuloSchedule] Support instructions with > 1 destination when walking… (authored by hgreving).
[ModuloSchedule] Support instructions with > 1 destination when walking…
Jun 8 2020, 12:09 PM

Jun 5 2020

hgreving created D81291: [ModuloSchedule] Support instructions with > 1 destination when walking canonical use..
Jun 5 2020, 11:47 AM · Restricted Project

May 29 2020

hgreving committed rGd8f2814c9138: [ModuloSchedule] Allow illegal phis to be moved across stages. (authored by hgreving).
[ModuloSchedule] Allow illegal phis to be moved across stages.
May 29 2020, 7:34 AM

May 28 2020

hgreving edited reviewers for D80678: [ModuloSchedule] Allow illegal phis to be moved across stages., added: kariddi; removed: marcello.maggioni.
May 28 2020, 9:49 AM · Restricted Project

May 27 2020

hgreving added a reviewer for D80678: [ModuloSchedule] Allow illegal phis to be moved across stages.: marcello.maggioni.
May 27 2020, 6:02 PM · Restricted Project
hgreving created D80678: [ModuloSchedule] Allow illegal phis to be moved across stages..
May 27 2020, 5:29 PM · Restricted Project
hgreving retitled D80678: [ModuloSchedule] Allow illegal phis to be moved across stages. from ModuloSchedule] Allow illegal phis to be moved across stages. to [ModuloSchedule] Allow illegal phis to be moved across stages..
May 27 2020, 5:29 PM · Restricted Project

May 21 2020

hgreving committed rG8a6a2c4cb66d: [ModuloSchedule] Add missing comma. (authored by hgreving).
[ModuloSchedule] Add missing comma.
May 21 2020, 1:33 PM

May 20 2020

hgreving accepted D79112: [SelectionDAG] Add the option of disabling generic combines..

LGTM!

May 20 2020, 2:54 PM · Restricted Project
hgreving added a comment to D79112: [SelectionDAG] Add the option of disabling generic combines..

nit: s/ didn't make any effect./ didn't have any effect./

May 20 2020, 2:53 PM · Restricted Project

May 15 2020

hgreving added a comment to D80027: Trivial fix for instruction with more than one destination in modulo peeler..

Thanks

May 15 2020, 12:31 PM · Restricted Project
hgreving created D80027: Trivial fix for instruction with more than one destination in modulo peeler..
May 15 2020, 12:31 PM · Restricted Project

May 7 2020

hgreving accepted D79581: [ModuloSchedule] Fix epilogue peeling with illegal phi..

LGTM!

May 7 2020, 9:05 AM · Restricted Project

Apr 16 2020

hgreving added a comment to D78175: [MachineDCE] Predicated instructions shouldn't clear LivePhysRegs..

LGTM

Apr 16 2020, 7:48 AM · Restricted Project
hgreving requested changes to D78175: [MachineDCE] Predicated instructions shouldn't clear LivePhysRegs..

s/kill the registers they right/kill the registers they write/

Apr 16 2020, 7:48 AM · Restricted Project

Apr 1 2020

hgreving added a comment to D74183: [IRGen] Add an alignment attribute to underaligned sret parameters.

If it's just tramp3d-v4, I'm not that concerned... but that's a weird result. On x86 in particular, alignment markings have almost no effect on optimization, generally.

I've just looked at the IR diff for tramp3d-v4 and it turns out that the root cause is an old friend of mine: The insertion of alignment assumptions during inlining (https://github.com/llvm/llvm-project/blob/b58902bc72c2b479b5ed27ec0d3422ba9782edbb/llvm/lib/Transforms/Utils/InlineFunction.cpp#L1139-L1173). That is, the IR now contains many instances of this sequence:

%ptrint = ptrtoint %class.GuardLayers* %guards_m to i64
%maskedptr = and i64 %ptrint, 3
%maskcond = icmp eq i64 %maskedptr, 0
tail call void @llvm.assume(i1 %maskcond)

to preserve the alignment information. From a cursory look I cannot tell whether these additional assumes also regress optimization (due to multi-use), but given the size increase on the final binary it seems pretty likely that this is the case.

This preservation of alignment during inlining is the reason why we used to not emit alignment information for pointer arguments in Rust for a long time: It caused serious regressions in optimization and increased compile-time. Nowadays we do emit alignment information, but set -preserve-alignment-assumptions-during-inlining=false to prevent this inlining behavior.

I think for the purposes of this revision, this means that we should probably either a) default preserve-alignment-assumptions-during-inlining to false (I would prefer this) or b) not emit the alignment unless it is smaller than the ABI alignment (I guess this was what this patch originally did?)

Apr 1 2020, 4:54 PM · Restricted Project

Feb 3 2020

hgreving added inline comments to D73804: [GVN] Add GVNOption to control load-pre more fine-grained..
Feb 3 2020, 3:24 PM · Restricted Project
hgreving updated the diff for D73804: [GVN] Add GVNOption to control load-pre more fine-grained..

llvm/test/Transforms/GVN/PRE/pre-load-in-loop.ll:
s/Should only be one load in the loop./Both loads should remain in the loop./

Feb 3 2020, 3:16 PM · Restricted Project
hgreving added inline comments to D73804: [GVN] Add GVNOption to control load-pre more fine-grained..
Feb 3 2020, 2:53 PM · Restricted Project
hgreving added inline comments to D73804: [GVN] Add GVNOption to control load-pre more fine-grained..
Feb 3 2020, 7:43 AM · Restricted Project

Jan 31 2020

hgreving added inline comments to D73804: [GVN] Add GVNOption to control load-pre more fine-grained..
Jan 31 2020, 2:35 PM · Restricted Project
hgreving added a comment to D73804: [GVN] Add GVNOption to control load-pre more fine-grained..

Hello, this switch intends to help our backend to software pipeline single basic block loops. The suppressed GVN opt is creating control flow in loops which we like to avoid. Thanks in advance for looking at the patch.

Jan 31 2020, 1:39 PM · Restricted Project
hgreving created D73804: [GVN] Add GVNOption to control load-pre more fine-grained..
Jan 31 2020, 1:30 PM · Restricted Project