Page MenuHomePhabricator

foad (Jay Foad)
User

Projects

User does not belong to any projects.

User Details

User Since
Oct 29 2014, 9:58 AM (324 w, 5 d)

Recent Activity

Today

foad added inline comments to D94746: [AMDGPU] Move kill lowering to WQM pass and add live mask tracking.
Mon, Jan 18, 7:30 AM · Restricted Project
foad added a reviewer for D94746: [AMDGPU] Move kill lowering to WQM pass and add live mask tracking: piotr.
Mon, Jan 18, 6:45 AM · Restricted Project
foad added a comment to D94768: [AMDGPU] Implement mir parseCustomPseudoSourceValue.

Isn't the point to stop using these meaningless names?

I don’t know, I just tried to stop and resume compilation with llc and it failed at these tokens.
What would the better way be?

Mon, Jan 18, 5:48 AM · Restricted Project
foad added inline comments to D88569: [DAGCombiner] Call SimplifyDemandedBits to simplify EXTRACT_VECTOR_ELT.
Mon, Jan 18, 3:40 AM · Restricted Project

Fri, Jan 15

foad added inline comments to D88569: [DAGCombiner] Call SimplifyDemandedBits to simplify EXTRACT_VECTOR_ELT.
Fri, Jan 15, 9:53 AM · Restricted Project
foad accepted D92219: [AMDGPU][GlobalISel] Avoid selecting S_PACK with constants.

LGTM.

Fri, Jan 15, 9:23 AM · Restricted Project
foad added reviewers for D94795: [AMDGPU] Fix use of HasModifiers in VopProfile: rampitec, arsenm.

"HasModifiers should not be true if at least one modifier is used."

Fri, Jan 15, 9:21 AM · Restricted Project
foad accepted D94532: [DAG] SimplifyDemandedBits - use KnownBits comparisons to remove ISD::UMIN/UMAX ops.

LGTM, thanks!

Fri, Jan 15, 9:08 AM · Restricted Project
foad added inline comments to D94746: [AMDGPU] Move kill lowering to WQM pass and add live mask tracking.
Fri, Jan 15, 7:24 AM · Restricted Project
foad added inline comments to D94747: [AMDGPU] Add llvm.amdgcn.wqm.demote intrinsic.
Fri, Jan 15, 6:40 AM · Restricted Project
foad added inline comments to D94748: [AMDGPU] Tidy up conditional branches from early termination.
Fri, Jan 15, 6:03 AM · Restricted Project
foad added inline comments to D94532: [DAG] SimplifyDemandedBits - use KnownBits comparisons to remove ISD::UMIN/UMAX ops.
Fri, Jan 15, 5:35 AM · Restricted Project
foad updated the diff for D88569: [DAGCombiner] Call SimplifyDemandedBits to simplify EXTRACT_VECTOR_ELT.

Rebase.

Fri, Jan 15, 3:53 AM · Restricted Project
foad added inline comments to D93963: [GlobalISel][AMDGPU] Lower G_UMULO/G_SMULO.
Fri, Jan 15, 2:15 AM · Restricted Project
foad added a comment to D87464: [TargetLowering] Improve SimplifyDemandedBits for AND and OR.

Reping

Fri, Jan 15, 1:49 AM · Restricted Project
foad added inline comments to D87464: [TargetLowering] Improve SimplifyDemandedBits for AND and OR.
Fri, Jan 15, 1:43 AM · Restricted Project
foad updated the diff for D87464: [TargetLowering] Improve SimplifyDemandedBits for AND and OR.

Rebase. Pass Op0/1DemandedBits into SimplifyMultipleUseDemandedBits.

Fri, Jan 15, 1:40 AM · Restricted Project

Thu, Jan 14

foad committed rG868da2ea939b: [SelectionDAG] Remove an early-out from computeKnownBits for smin/smax (authored by foad).
[SelectionDAG] Remove an early-out from computeKnownBits for smin/smax
Thu, Jan 14, 10:16 AM
foad closed D87145: [SelectionDAG] Remove an early-out from computeKnownBits for smin/smax.
Thu, Jan 14, 10:15 AM · Restricted Project
foad updated the diff for D87145: [SelectionDAG] Remove an early-out from computeKnownBits for smin/smax.

Rebase.

Thu, Jan 14, 10:08 AM · Restricted Project
foad added inline comments to D94693: Improve KnownBits analyses for SMIN/SMAX DAG nodes..
Thu, Jan 14, 9:55 AM · Restricted Project
foad updated the diff for D87145: [SelectionDAG] Remove an early-out from computeKnownBits for smin/smax.

Rebase. D87236 seems to have fixed the code quality regressions.

Thu, Jan 14, 8:44 AM · Restricted Project
foad added a comment to D94693: Improve KnownBits analyses for SMIN/SMAX DAG nodes..

This is very similar to D87145.

Thu, Jan 14, 8:08 AM · Restricted Project
foad added inline comments to D94532: [DAG] SimplifyDemandedBits - use KnownBits comparisons to remove ISD::UMIN/UMAX ops.
Thu, Jan 14, 7:28 AM · Restricted Project
foad added a comment to rGc0939fddf80c: [Support] Simplify KnownBits::sextInReg implementation..

Thanks!

Thu, Jan 14, 7:19 AM
foad committed rG90b310f6caf0: [Support] Simplify KnownBits::icmp helpers. NFC. (authored by foad).
[Support] Simplify KnownBits::icmp helpers. NFC.
Thu, Jan 14, 6:23 AM
foad committed rG517196e56912: [Analysis,CodeGen] Make use of KnownBits::makeConstant. NFC. (authored by foad).
[Analysis,CodeGen] Make use of KnownBits::makeConstant. NFC.
Thu, Jan 14, 6:23 AM
foad committed rGa1cba5b7a1fb: [SelectionDAG] Make use of KnownBits::commonBits. NFC. (authored by foad).
[SelectionDAG] Make use of KnownBits::commonBits. NFC.
Thu, Jan 14, 6:23 AM
foad closed D94595: [Support] Simplify KnownBits::icmp helpers. NFC..
Thu, Jan 14, 6:23 AM · Restricted Project
foad closed D94588: [Analysis,CodeGen] Make use of KnownBits::makeConstant. NFC..
Thu, Jan 14, 6:23 AM · Restricted Project
foad closed D94587: [SelectionDAG] Make use of KnownBits::commonBits. NFC..
Thu, Jan 14, 6:23 AM · Restricted Project
foad accepted D94416: [PM] Avoid duplicates in the Used/Preserved/Required sets.

Thanks for doing the timings. The patch seems reasonable to me. I'll approve it but it would be great if you could find at least one other reviewer to take a look too.

Thu, Jan 14, 2:47 AM · Restricted Project

Wed, Jan 13

foad added inline comments to D94595: [Support] Simplify KnownBits::icmp helpers. NFC..
Wed, Jan 13, 7:22 AM · Restricted Project
foad added inline comments to D94153: [AMDGPU][Inliner] Remove amdgpu-inline and add new TTI inline hooks.
Wed, Jan 13, 7:11 AM · Restricted Project
foad added inline comments to D94153: [AMDGPU][Inliner] Remove amdgpu-inline and add new TTI inline hooks.
Wed, Jan 13, 7:05 AM · Restricted Project
foad added a comment to D94532: [DAG] SimplifyDemandedBits - use KnownBits comparisons to remove ISD::UMIN/UMAX ops.

LGTM. Any reason not to do this for SMAX and SMIN too?

Wed, Jan 13, 6:22 AM · Restricted Project
foad added a comment to rG23b41986527a: [Support] Add KnownBits::icmp helpers..

@foad Can you raise a patch please?

Wed, Jan 13, 5:55 AM
foad requested review of D94595: [Support] Simplify KnownBits::icmp helpers. NFC..
Wed, Jan 13, 5:54 AM · Restricted Project
foad added inline comments to rG23b41986527a: [Support] Add KnownBits::icmp helpers..
Wed, Jan 13, 4:19 AM
foad added inline comments to rG6895581fd2c1: [Support] Add KnownBits::getSignedMinValue/getSignedMaxValue helpers..
Wed, Jan 13, 4:12 AM
foad added a comment to rG9cf4f493a72f: [DAG] Move SelectionDAG implementation to KnownBits::setInReg(). NFCI..

This could do with an exhaustive unit test.

Wed, Jan 13, 4:06 AM
foad added inline comments to rG899674274124: [KnownBits] Add KnownBits::makeConstant helper. NFCI..
Wed, Jan 13, 3:43 AM
foad requested review of D94588: [Analysis,CodeGen] Make use of KnownBits::makeConstant. NFC..
Wed, Jan 13, 3:36 AM · Restricted Project
foad requested review of D94587: [SelectionDAG] Make use of KnownBits::commonBits. NFC..
Wed, Jan 13, 3:14 AM · Restricted Project

Tue, Jan 12

foad added a comment to D94416: [PM] Avoid duplicates in the Used/Preserved/Required sets.

I think improving the legacy pass manager is fine (after all it is taking a very long time for the new pass manager to supersede it). But I think any patch that claims to do less work overall, or speed something up, needs *some* kind of evidence that it actually has the desired effect.

Tue, Jan 12, 9:51 AM · Restricted Project
foad added a reviewer for D92309: [LegacyPM] Update InversedLastUser on the fly. NFC.: bjope.
Tue, Jan 12, 9:10 AM · Restricted Project
foad added a comment to D94416: [PM] Avoid duplicates in the Used/Preserved/Required sets.

Does this make any difference in practice? E.g. does the output of opt -O1 -debug-pass=Executions change, or can you measure any timing difference?

Tue, Jan 12, 9:02 AM · Restricted Project
foad closed D86618: [AMDGPU][GlobalISel] Remove some duplicate RUN lines.
Tue, Jan 12, 3:04 AM · Restricted Project
foad committed rG794e3d94d5a9: [AMDGPU][GlobalISel] Remove some duplicate RUN lines (authored by foad).
[AMDGPU][GlobalISel] Remove some duplicate RUN lines
Tue, Jan 12, 3:04 AM
foad committed rGf264f9ad7df5: [SlotIndexes] Fix and simplify basic block splitting (authored by foad).
[SlotIndexes] Fix and simplify basic block splitting
Tue, Jan 12, 3:03 AM
foad closed D94311: [SlotIndexes] Fix and simplify basic block splitting.
Tue, Jan 12, 3:03 AM · Restricted Project

Mon, Jan 11

foad accepted D94406: [AMDGPU] Fix failing assert with scratch ST mode.

LGTM with comment fix.

Mon, Jan 11, 7:13 AM · Restricted Project
foad accepted D94341: [AMDGPU] Add _e64 suffix to VOP3 Insts.

Looks OK to me. Does this enable any immediate simplifications? Or is it just to help with future work?

Mon, Jan 11, 7:08 AM · Restricted Project
foad added a comment to D94406: [AMDGPU] Fix failing assert with scratch ST mode.

I think the intention was that getMemOperandsWithOffsetWidth should return false if no base operands were found (perhaps MachineScheduler could assert this?)

I’d argue that in this case, we found the base operands and we know that except for the constant offset, they are zero.
Also, we want these operations to be claused, I think this is the likely case for stack accesses in entry-points.

Mon, Jan 11, 6:21 AM · Restricted Project
foad added a comment to D94341: [AMDGPU] Add _e64 suffix to VOP3 Insts.

Maybe reword the description to make it clear that it only affects the names of pseudos, not real instructions, assuming I got that right?

Mon, Jan 11, 5:50 AM · Restricted Project
foad added a comment to D94406: [AMDGPU] Fix failing assert with scratch ST mode.

I think the intention was that getMemOperandsWithOffsetWidth should return false if no base operands were found (perhaps MachineScheduler could assert this?): https://reviews.llvm.org/source/llvm-github/browse/main/llvm/include/llvm/CodeGen/TargetInstrInfo.h$1304

Mon, Jan 11, 5:44 AM · Restricted Project
foad committed rG6dcf9207df11: [AMDGPU] Fix a urem combine test to test what it was supposed to (authored by foad).
[AMDGPU] Fix a urem combine test to test what it was supposed to
Mon, Jan 11, 5:34 AM
foad added a comment to D86618: [AMDGPU][GlobalISel] Remove some duplicate RUN lines.

Is this still needed?

Mon, Jan 11, 4:14 AM · Restricted Project
foad accepted D88777: [AMDGPU] Add SI_EARLY_TERMINATE_SCC0 for early terminating shader.

LGTM.

Mon, Jan 11, 2:47 AM · Restricted Project

Fri, Jan 8

foad added inline comments to D91066: [SlotIndexes] Consider existing index range in insertMBBIntoMaps.
Fri, Jan 8, 8:05 AM · Restricted Project
foad added a comment to D94311: [SlotIndexes] Fix and simplify basic block splitting.

This is intended to supersede D91066 and D91064.

Fri, Jan 8, 8:03 AM · Restricted Project
foad requested review of D94311: [SlotIndexes] Fix and simplify basic block splitting.
Fri, Jan 8, 8:03 AM · Restricted Project
foad added a comment to D88777: [AMDGPU] Add SI_EARLY_TERMINATE_SCC0 for early terminating shader.

Patch looks fine to me. Why is it useful to terminate on scc0 instead of scc1, or is it an arbitrary choice? Could you give a slightly more realistic example of how it would be used? Your tests all have:

S_CMP_EQ_U32 killed $sgpr0, 0, implicit-def $scc
SI_EARLY_TERMINATE_SCC0 implicit $scc, implicit $exec

which will terminate early if sgpr0 is not 0. Is the idea that this would be used when we're just about to AND sgpr0 into exec?

Fri, Jan 8, 2:08 AM · Restricted Project

Thu, Jan 7

foad added inline comments to D94203: GlobalISel: Add combine for G_UREM by power of 2.
Thu, Jan 7, 6:45 AM · Restricted Project
foad accepted D94203: GlobalISel: Add combine for G_UREM by power of 2.

LGTM.

Thu, Jan 7, 3:16 AM · Restricted Project

Wed, Jan 6

foad added inline comments to D94138: Require chained analyses in BasicAA and AAResults to be transitive.
Wed, Jan 6, 6:35 AM · Restricted Project
foad accepted D94146: AMDGPU/GlobalISel: Add baseline IR tests for fdiv.

Seems obvious.

Wed, Jan 6, 2:48 AM · Restricted Project
foad accepted D94145: AMDGPU/GlobalISel: Update fdiv lowering for denormal/ulp interaction.

Looks good but "to match 884acbb9e167d5668e43581630239d688edec8ad" might be more accurate since that was where the current definition of AllowInaccurateRcp was introduced.

Wed, Jan 6, 2:40 AM · Restricted Project
foad accepted D94150: AMDGPU: Explicitly use SelectionDAG in legacy intrinsic tests.

Seems pretty obvious. Is it well defined which intrinsics are "legacy"? E.g. is it documented anywhere, or do we ever generate warnings for them or anything like that?

Wed, Jan 6, 1:28 AM · Restricted Project

Tue, Jan 5

foad added a comment to D87870: [GISel] Add new combines for G_FMUL.

Well (fmul x, 0.0) -> 0.0 is still wrong if x is Inf or NaN.

Tue, Jan 5, 11:59 AM · Restricted Project
foad added a comment to D94102: [AMDGPU] Deduplicate VOP tablegen asm & ins.

I don't really follow the details but it looks like quite a nice simplification.

Tue, Jan 5, 11:04 AM · Restricted Project
foad added inline comments to D92219: [AMDGPU][GlobalISel] Avoid selecting S_PACK with constants.
Tue, Jan 5, 10:30 AM · Restricted Project
foad added inline comments to D93708: [AMDGPU] Add a new Clamp Pattern to the GlobalISel Path..
Tue, Jan 5, 4:34 AM · Restricted Project, Restricted Project
foad committed rG3914bebe91f6: [AMDGPU] Handle v_fmac_legacy_f32 in SIFoldOperands (authored by foad).
[AMDGPU] Handle v_fmac_legacy_f32 in SIFoldOperands
Tue, Jan 5, 4:08 AM
foad committed rG639a50e2f138: [AMDGPU] Precommit test case for D94010 (authored by foad).
[AMDGPU] Precommit test case for D94010
Tue, Jan 5, 4:08 AM
foad closed D94010: [AMDGPU] Handle v_fmac_legacy_f32 in SIFoldOperands.
Tue, Jan 5, 4:08 AM · Restricted Project
foad committed rG4e6054a86c0c: [AMDGPU] Split out new helper function macToMad in SIFoldOperands. NFC. (authored by foad).
[AMDGPU] Split out new helper function macToMad in SIFoldOperands. NFC.
Tue, Jan 5, 3:55 AM
foad closed D94009: [AMDGPU] Split out new helper function macToMad in SIFoldOperands. NFC..
Tue, Jan 5, 3:55 AM · Restricted Project
foad updated the diff for D94009: [AMDGPU] Split out new helper function macToMad in SIFoldOperands. NFC..

Use INSTRUCTION_LIST_END.

Tue, Jan 5, 3:53 AM · Restricted Project

Mon, Jan 4

foad added a comment to D94010: [AMDGPU] Handle v_fmac_legacy_f32 in SIFoldOperands.

The point of this patch is to remove some unnecessary movs around the v_fma(c)_legacy_f32 instructions. I can pre-commit the new test case to make this clearer if you're happy with it.

Mon, Jan 4, 3:05 AM · Restricted Project
foad requested review of D94010: [AMDGPU] Handle v_fmac_legacy_f32 in SIFoldOperands.
Mon, Jan 4, 3:04 AM · Restricted Project
foad requested review of D94009: [AMDGPU] Split out new helper function macToMad in SIFoldOperands. NFC..
Mon, Jan 4, 3:03 AM · Restricted Project

Wed, Dec 23

foad added a reviewer for D93708: [AMDGPU] Add a new Clamp Pattern to the GlobalISel Path.: Petar.Avramovic.
Wed, Dec 23, 2:42 AM · Restricted Project, Restricted Project

Dec 18 2020

foad accepted D91336: AMDGPU/GlobalISel: Fix negative offset folding for buffer_load.

LGTM. It is correct in more cases than it was before, and it mostly matches what the DAG version does.

Dec 18 2020, 6:04 AM · Restricted Project

Dec 17 2020

foad added reviewers for D92219: [AMDGPU][GlobalISel] Avoid selecting S_PACK with constants: aemerson, dsanders, qcolombet, paquette, aditya_nandakumar, gargaroff.

I think this looks good, and we could even get rid of the AnyExtAsZext flag and do it unconditionally. Adding some globalisel folks to see if there are any objections.

Dec 17 2020, 3:44 AM · Restricted Project

Dec 16 2020

foad accepted D93394: CodeGen: Move function to get subregister indexes to cover a LaneMask.

Seems like an obvious refactoring to me. I think the API could be simplified though. How about returning all indices via the SmallVector, and returning a bool failure indication? Or say that if the vector is empty, it means failure?

Dec 16 2020, 6:20 AM · Restricted Project
foad added a comment to rGdb48a6de7702: [RISCV] Define vwadd/vwaddu/vwsub/vwsubu intrinsics..

This seems to cause a whole load of warnings during my Release build:

[1148/2221] Building RISCVGenAsmMatcher.inc...
warning: Compiler has made implicit assumption that TypeSize is not scalable. This may or may not lead to broken code.
[1793/2221] Building RISCVGenAsmWriter.inc...
warning: Compiler has made implicit assumption that TypeSize is not scalable. This may or may not lead to broken code.
[1795/2221] Building RISCVGenCompressInstEmitter.inc...
warning: Compiler has made implicit assumption that TypeSize is not scalable. This may or may not lead to broken code.
[1802/2221] Building RISCVGenRegisterBank.inc...
warning: Compiler has made implicit assumption that TypeSize is not scalable. This may or may not lead to broken code.
[1803/2221] Building RISCVGenRegisterInfo.inc...
warning: Compiler has made implicit assumption that TypeSize is not scalable. This may or may not lead to broken code.
[1805/2221] Building RISCVGenDAGISel.inc...
warning: Compiler has made implicit assumption that TypeSize is not scalable. This may or may not lead to broken code.
[1810/2221] Building RISCVGenGlobalISel.inc...
warning: Compiler has made implicit assumption that TypeSize is not scalable. This may or may not lead to broken code.
[1813/2221] Building RISCVGenInstrInfo.inc...
warning: Compiler has made implicit assumption that TypeSize is not scalable. This may or may not lead to broken code.
Dec 16 2020, 6:08 AM

Dec 15 2020

foad accepted D93287: [AMDGPU] Unify flat offset logic.
Dec 15 2020, 3:07 AM · Restricted Project

Dec 14 2020

foad committed rG07e92e6b6002: [AMDGPU] Make use of HasSMemRealTime predicate. NFC. (authored by foad).
[AMDGPU] Make use of HasSMemRealTime predicate. NFC.
Dec 14 2020, 8:43 AM
foad closed D93202: [AMDGPU] Make use of HasSMemRealTime predicate. NFC..
Dec 14 2020, 8:43 AM · Restricted Project
foad added inline comments to D91336: AMDGPU/GlobalISel: Fix negative offset folding for buffer_load.
Dec 14 2020, 7:33 AM · Restricted Project
foad requested review of D93202: [AMDGPU] Make use of HasSMemRealTime predicate. NFC..
Dec 14 2020, 3:42 AM · Restricted Project
foad accepted D93187: [AMDGPU][NFC] Rename opsel/opsel_hi/neg_lo/neg_hi with suffix 0.

Sounds good to me.

Dec 14 2020, 1:23 AM · Restricted Project
foad accepted D93188: [AMDGPU][NFC] Remove unused VOP3Mods0Clamp.

I wouldn't have bothered with pre-commit review for a change like this.

Dec 14 2020, 1:22 AM · Restricted Project

Dec 11 2020

foad committed rG4f25e5398211: [AMDGPU] Make use of emitRemovedIntrinsicError. NFC. (authored by foad).
[AMDGPU] Make use of emitRemovedIntrinsicError. NFC.
Dec 11 2020, 6:03 AM

Dec 10 2020

foad accepted D92767: [AMDGPU] Resolve issues when picking between ds_read/write and ds_read2/write2.

Looks good, thanks!

Dec 10 2020, 3:34 AM · Restricted Project
foad added a comment to D84165: GlobalISel: Look through G_ANYEXT and G_IMPLICIT_DEF for constants.

Reverse ping! Was there a reason not to apply this?

Dec 10 2020, 3:04 AM · Restricted Project

Dec 8 2020

foad committed rG03663e4130d7: [AMDGPU] Add occupancy level tests for GFX10.3. NFC. (authored by foad).
[AMDGPU] Add occupancy level tests for GFX10.3. NFC.
Dec 8 2020, 6:27 AM
foad closed D92839: [AMDGPU] Add occupancy level tests for GFX10.3. NFC..
Dec 8 2020, 6:27 AM · Restricted Project
foad requested review of D92839: [AMDGPU] Add occupancy level tests for GFX10.3. NFC..
Dec 8 2020, 5:18 AM · Restricted Project