Page MenuHomePhabricator

frasercrmck (Fraser Cormack)
User

Projects

User does not belong to any projects.

User Details

User Since
Jun 1 2015, 8:58 AM (316 w, 23 h)

Recent Activity

Today

frasercrmck committed rGc97cf73867dd: [Utils][vim] Add missing highlights for fast-math flags (authored by frasercrmck).
[Utils][vim] Add missing highlights for fast-math flags
Tue, Jun 22, 1:48 AM
frasercrmck closed D104541: [Utils][vim] Add missing highlights for fast-math flags.
Tue, Jun 22, 1:47 AM · Restricted Project
frasercrmck updated the summary of D104541: [Utils][vim] Add missing highlights for fast-math flags.
Tue, Jun 22, 1:09 AM · Restricted Project
frasercrmck updated the diff for D104541: [Utils][vim] Add missing highlights for fast-math flags.
  • fix alphabetization(?) of fneg
Tue, Jun 22, 1:09 AM · Restricted Project

Yesterday

frasercrmck added a comment to D102852: [RISCV] Fix a crash when lowering split float arguments.

Some thoughts:

  1. I agree with you that the psABI doesn't (for now, at least) specify this ABI condition so technically we can do whatever we want. Whether we should still follow the spirit of the ABI or be free to make something completely different I'm not sure about. As you noted, this IR construct might arguably also be outside of what a vector-aware ABI would specify (but clang isn't the only frontend).
  1. Besides the psABI not covering vectors explicitly, I think what makes the situation more confusing is that arrays are passed by reference, so the specification of the ABI for non-scalar types ends up being less general than it could be. IMO vectors passed by value are reasonably analogous to the "Floating-point reals" and other structs, the way they are discussed in the spec.

Since the Hardware Floating-point Calling Convention doesn't refer to "aggregates" as much as it refers to "structs" I suppose it follows from your opinion about aggregates that, if the psABI is relevant, we should be looking entirely at the Integer Calling Convention? So there shouldn't be any use of floating-point registers in lowering these illegal vectors at all, no matter which ABI is used?

A struct is an aggregate. I think the FP section just happens to use more specific language, as it details the rules for the actual language constructs (structs, reals, etc) that would be impacted by the FP calling convention. But if we decided to follow the spirit of the psABI my overall reading of it is that I would expect small (<= 2×XLEN) vectors passed by value to follow the FP convention, when available.

I'm not opposed to this implementation as "something that works" and that "doesn't have to be compatible with anything" and that "solves our problems now". It's just not the ABI I would expect. It seemed to me like an unnecessary departure from convention, and that's why I didn't find it particularly appealing. But perhaps it doesn't matter? In any case, thanks for the patch and for your analysis. Let's hear from other people -- @kito-cheng @asb and others.

Mon, Jun 21, 7:17 AM · Restricted Project
frasercrmck added a comment to D104308: [VP] Add vector-predicated reduction intrinsics.

Disabling lanes is really what makes the difference between these and the regular reduction intrinsics.
There is also the corner case that all lanes are disabled and i am unsure what the return value should be then. Any thoughts on that?

Mon, Jun 21, 6:17 AM · Restricted Project
frasercrmck updated the summary of D102852: [RISCV] Fix a crash when lowering split float arguments.
Mon, Jun 21, 5:14 AM · Restricted Project
frasercrmck added a comment to D102852: [RISCV] Fix a crash when lowering split float arguments.

Sorry if I'm misunderstanding your point, but we *are* accessing 0-63(sp) (later below).

Mon, Jun 21, 5:14 AM · Restricted Project
frasercrmck committed rG983972bfb0f9: [VP][NFCI] Address various clang-tidy warnings (authored by frasercrmck).
[VP][NFCI] Address various clang-tidy warnings
Mon, Jun 21, 3:06 AM
frasercrmck closed D104288: [VP][NFCI] Address various clang-tidy warnings.
Mon, Jun 21, 3:06 AM · Restricted Project
frasercrmck added a comment to D104612: [CGP][RISCV] Teach CodeGenPrepare::optimizeSwitchInst to honor isSExtCheaperThanZExt..

Seems reasonable to me but I'll let others take a look

Mon, Jun 21, 2:07 AM · Restricted Project
frasercrmck accepted D104163: [RISCV] Add isel patterns to match vmacc/vmadd/vnmsub/vnmsac from add/sub and mul..

LGTM, cheers!

Mon, Jun 21, 2:06 AM · Restricted Project
frasercrmck added inline comments to D104541: [Utils][vim] Add missing highlights for fast-math flags.
Mon, Jun 21, 2:00 AM · Restricted Project
frasercrmck updated the diff for D104288: [VP][NFCI] Address various clang-tidy warnings.
  • undo unrelated change
Mon, Jun 21, 1:59 AM · Restricted Project
frasercrmck updated the diff for D104541: [Utils][vim] Add missing highlights for fast-math flags.
  • reflow to 80 cols
Mon, Jun 21, 1:57 AM · Restricted Project
frasercrmck updated the summary of D104541: [Utils][vim] Add missing highlights for fast-math flags.
Mon, Jun 21, 1:57 AM · Restricted Project
frasercrmck updated the diff for D104288: [VP][NFCI] Address various clang-tidy warnings.

reflow to 80 cols

Mon, Jun 21, 1:54 AM · Restricted Project
frasercrmck updated the diff for D104288: [VP][NFCI] Address various clang-tidy warnings.
  • rebase
Mon, Jun 21, 1:36 AM · Restricted Project

Fri, Jun 18

frasercrmck requested review of D104541: [Utils][vim] Add missing highlights for fast-math flags.
Fri, Jun 18, 8:40 AM · Restricted Project
frasercrmck added a comment to D104308: [VP] Add vector-predicated reduction intrinsics.

I wonder whether the semantics sections in the documentation should just refer back to semantics sections of the regular reduction intrinsics instead of replicating them. In the end, we use those when vp reduction are expanded anyway: if standard reductions switch semantics at some point, we will too unwittingly.

Fri, Jun 18, 4:07 AM · Restricted Project
frasercrmck updated the diff for D104308: [VP] Add vector-predicated reduction intrinsics.
  • rebase
  • remove debug print
  • move isVPReduction into VPReductionIntrinsic
  • flesh out expand-vp.ll test
Fri, Jun 18, 4:05 AM · Restricted Project
frasercrmck added inline comments to D104163: [RISCV] Add isel patterns to match vmacc/vmadd/vnmsub/vnmsac from add/sub and mul..
Fri, Jun 18, 2:59 AM · Restricted Project
frasercrmck added inline comments to D104288: [VP][NFCI] Address various clang-tidy warnings.
Fri, Jun 18, 2:12 AM · Restricted Project
frasercrmck updated the diff for D104288: [VP][NFCI] Address various clang-tidy warnings.

Use FunctionType::param_iterator over auto.

Fri, Jun 18, 2:11 AM · Restricted Project
frasercrmck accepted D104069: [RISCV] Teach vsetvli insertion to remember when predecessors have same AVL and SEW/LMUL ratio if their VTYPEs otherwise mismatch..

LGTM, thanks!

Fri, Jun 18, 12:39 AM · Restricted Project

Thu, Jun 17

frasercrmck added a reviewer for D102852: [RISCV] Fix a crash when lowering split float arguments: kito-cheng.

The last time I looked at this I had some concerns but I didn't have time to fully investigate them.
To help get this going again, could you please clarify:

  • Is this patch and/or the ABI changes supposed to be (roughly) permanent or just a temporary fix/workaround?
  • Can you provide more details about the "The register arguments seem to occupy unused stack slots, which I've observed in other vector tests"?
Thu, Jun 17, 8:50 AM · Restricted Project
frasercrmck added a comment to D104237: [RISCV][VP] Lower FP VP ISD nodes to RVV instructions.

Thanks for looking into it. In our downstream we defer it too to a function call so we can complete the compilation and elsewhere we can provide an implementation (there is a number of other ISD nodes that we already lower this way like exp, sin, cos, etc.). AFAICT Arm SVE is not lowering any operation to LibCallyet so perhaps this conversation should be raised in the mailing list. Not sure if we need to align compiler-rt/libgcc in terms of naming conventions here.

I'm happy to land this without frem.

Thu, Jun 17, 2:28 AM · Restricted Project
frasercrmck committed rGfed1503e855a: [RISCV][VP] Lower FP VP ISD nodes to RVV instructions (authored by frasercrmck).
[RISCV][VP] Lower FP VP ISD nodes to RVV instructions
Thu, Jun 17, 2:12 AM
frasercrmck closed D104237: [RISCV][VP] Lower FP VP ISD nodes to RVV instructions.
Thu, Jun 17, 2:12 AM · Restricted Project
frasercrmck added inline comments to D102842: [Verifier] Fail on invalid indices for {insert,extract} vector intrinsics.
Thu, Jun 17, 1:39 AM · Restricted Project

Wed, Jun 16

frasercrmck added inline comments to D104288: [VP][NFCI] Address various clang-tidy warnings.
Wed, Jun 16, 11:18 PM · Restricted Project
frasercrmck added a comment to D104237: [RISCV][VP] Lower FP VP ISD nodes to RVV instructions.

Many targets scalarize frem on fixed vectors. It looks like frem on scalable vectors currently crashes on SVE because the type legalizer can't unroll it.

Indeed. I don't think we or any target support any level of scalable-vector frem right now, so I think this discussion is orthogonal to this VP patch. But I'll spend some time today seeing if there's anything we can do since it's come up.

Wed, Jun 16, 9:01 AM · Restricted Project
frasercrmck added a comment to D104237: [RISCV][VP] Lower FP VP ISD nodes to RVV instructions.

Many targets scalarize frem on fixed vectors. It looks like frem on scalable vectors currently crashes on SVE because the type legalizer can't unroll it.

Wed, Jun 16, 2:45 AM · Restricted Project
frasercrmck added inline comments to D104069: [RISCV] Teach vsetvli insertion to remember when predecessors have same AVL and SEW/LMUL ratio if their VTYPEs otherwise mismatch..
Wed, Jun 16, 2:26 AM · Restricted Project

Tue, Jun 15

frasercrmck added a comment to D104237: [RISCV][VP] Lower FP VP ISD nodes to RVV instructions.

Will frem need to be expanded to a loop in the expansion pass?

Would it be reasonable to expand it using the definition of fmod but vector-wise? It seems we'd need a division, a rounding to integer towards zero, convert back to float, a mutiplication and then a subtraction.

Tue, Jun 15, 10:32 AM · Restricted Project
frasercrmck added inline comments to D104308: [VP] Add vector-predicated reduction intrinsics.
Tue, Jun 15, 9:46 AM · Restricted Project
frasercrmck added inline comments to D104308: [VP] Add vector-predicated reduction intrinsics.
Tue, Jun 15, 9:43 AM · Restricted Project
frasercrmck requested review of D104308: [VP] Add vector-predicated reduction intrinsics.
Tue, Jun 15, 9:41 AM · Restricted Project
frasercrmck added a reviewer for D104237: [RISCV][VP] Lower FP VP ISD nodes to RVV instructions: vkmr.
Tue, Jun 15, 3:40 AM · Restricted Project
frasercrmck requested review of D104288: [VP][NFCI] Address various clang-tidy warnings.
Tue, Jun 15, 3:39 AM · Restricted Project
frasercrmck added a comment to D99355: Implementation of intrinsic and SDNode definitions for VP load, store, gather, scatter..

Just so I know for the VP reduction intrinsics, should we not be adding LangRef documentation at this stage too, given we're adding new intrinsics? How about support in the ExpandVectorPredication pass? Should that come now or can it wait?

Ideally, yes, you want to have the documentation with the intrinsics and their expansion in one patch.
If that grows too large, you split off the expansion for another patch.
For this patch, since we are kind of stalling on memory intrinsics to become available upstream, it'd be ok for me to just have the intrinsics and save everything else for followup patches. The intrinsics are documented in the reference implementation anyway.

Tue, Jun 15, 1:54 AM · Restricted Project, Restricted Project

Mon, Jun 14

frasercrmck committed rGc75e454cb932: [RISCV] Transform unaligned RVV vector loads/stores to aligned ones (authored by frasercrmck).
[RISCV] Transform unaligned RVV vector loads/stores to aligned ones
Mon, Jun 14, 10:20 AM
frasercrmck closed D104032: [RISCV] Transform unaligned RVV vector loads/stores to aligned ones.
Mon, Jun 14, 10:20 AM · Restricted Project
frasercrmck added inline comments to D104032: [RISCV] Transform unaligned RVV vector loads/stores to aligned ones.
Mon, Jun 14, 9:22 AM · Restricted Project
frasercrmck updated the diff for D104032: [RISCV] Transform unaligned RVV vector loads/stores to aligned ones.
  • rebase
  • use cast over dyn_cast
Mon, Jun 14, 9:22 AM · Restricted Project
frasercrmck requested review of D104237: [RISCV][VP] Lower FP VP ISD nodes to RVV instructions.
Mon, Jun 14, 9:14 AM · Restricted Project
frasercrmck added a comment to D99355: Implementation of intrinsic and SDNode definitions for VP load, store, gather, scatter..

Just so I know for the VP reduction intrinsics, should we not be adding LangRef documentation at this stage too, given we're adding new intrinsics? How about support in the ExpandVectorPredication pass? Should that come now or can it wait?

Mon, Jun 14, 3:59 AM · Restricted Project, Restricted Project

Fri, Jun 11

frasercrmck committed rG71a02ddda105: [VP][NFC] Format comment to 80 columns (authored by frasercrmck).
[VP][NFC] Format comment to 80 columns
Fri, Jun 11, 5:02 AM

Thu, Jun 10

frasercrmck requested review of D104032: [RISCV] Transform unaligned RVV vector loads/stores to aligned ones.
Thu, Jun 10, 7:31 AM · Restricted Project
frasercrmck accepted D103622: [RISCV] Avoid scalar outgoing arguments overwriting vector frame objects..

LGTM other than my last question. Thanks!

Thu, Jun 10, 1:57 AM · Restricted Project

Wed, Jun 9

frasercrmck added a comment to D99355: Implementation of intrinsic and SDNode definitions for VP load, store, gather, scatter..

LGTM other than the last few nits.

Wed, Jun 9, 7:48 AM · Restricted Project, Restricted Project
frasercrmck added a comment to D103622: [RISCV] Avoid scalar outgoing arguments overwriting vector frame objects..

Thanks for the detailed explanation.

Wed, Jun 9, 7:01 AM · Restricted Project
frasercrmck added a comment to D103758: [RISCV] Use ZeroOrNegativeOneBooleanContent for vectors..

I'm not familiar enough with all the decisions this affects but I agree that it should be the same. Have you seen any changes introduced by this? I'm assuming you may have tried it downstream, for example.

Wed, Jun 9, 6:45 AM · Restricted Project
frasercrmck committed rG502edebd9d6e: [ValueTypes][RISCV] Cap RVV fixed-length vectors by size (authored by frasercrmck).
[ValueTypes][RISCV] Cap RVV fixed-length vectors by size
Wed, Jun 9, 4:24 AM
frasercrmck closed D103884: [ValueTypes][RISCV] Cap RVV fixed-length vectors by size.
Wed, Jun 9, 4:23 AM · Restricted Project
frasercrmck updated the diff for D103884: [ValueTypes][RISCV] Cap RVV fixed-length vectors by size.
  • rebase on top of new MVT formatting
  • adjust tests for new VType formatting
Wed, Jun 9, 2:19 AM · Restricted Project
frasercrmck added a comment to D103790: [RISCV][NFC] Add a single space after comma for VType.

Note that I pushed rG292f4197249b to fix the failing tests introduced by this patch. Please check the test results before merging in the future, e.g. https://reviews.llvm.org/B107914 was showing this issue.

Wed, Jun 9, 1:59 AM · Restricted Project
frasercrmck committed rG292f4197249b: [RISCV] Fix failing RVV MC tests (authored by frasercrmck).
[RISCV] Fix failing RVV MC tests
Wed, Jun 9, 1:55 AM
frasercrmck committed rG80d556441adc: [ValueTypes] Add missing enum names for MVTs (authored by frasercrmck).
[ValueTypes] Add missing enum names for MVTs
Wed, Jun 9, 1:50 AM
frasercrmck closed D103883: [ValueTypes] Add missing enum names for MVTs.
Wed, Jun 9, 1:49 AM · Restricted Project
frasercrmck committed rGe8f1f8910313: [RISCV] Support CONCAT_VECTORS on scalable masks (authored by frasercrmck).
[RISCV] Support CONCAT_VECTORS on scalable masks
Wed, Jun 9, 1:16 AM
frasercrmck closed D103896: [RISCV] Support CONCAT_VECTORS on scalable masks.
Wed, Jun 9, 1:16 AM · Restricted Project

Tue, Jun 8

frasercrmck requested review of D103896: [RISCV] Support CONCAT_VECTORS on scalable masks.
Tue, Jun 8, 7:06 AM · Restricted Project
frasercrmck added a comment to D103881: [ValueTypes] Define MVTs for v6i32, v6f32, v7i32, v7f32.

Awfully brave to add new non-pow2 types; I like it. Unfortunate that we've both filed patches to this same file at the same time (D103884). One of us will have a fun time.

Tue, Jun 8, 4:05 AM · Restricted Project
frasercrmck updated the diff for D103884: [ValueTypes][RISCV] Cap RVV fixed-length vectors by size.
  • update code comments
Tue, Jun 8, 4:02 AM · Restricted Project
frasercrmck requested review of D103884: [ValueTypes][RISCV] Cap RVV fixed-length vectors by size.
Tue, Jun 8, 3:55 AM · Restricted Project
frasercrmck requested review of D103883: [ValueTypes] Add missing enum names for MVTs.
Tue, Jun 8, 3:44 AM · Restricted Project
frasercrmck committed rGccd1e087f370: [RISCV] Add a test case showing inefficient vector codegen (authored by frasercrmck).
[RISCV] Add a test case showing inefficient vector codegen
Tue, Jun 8, 3:15 AM
frasercrmck accepted D102686: [VP] getDeclarationForParams.

Might as well throw my hat in here, LGTM.

Tue, Jun 8, 2:01 AM · Restricted Project, Restricted Project
frasercrmck accepted D103736: [RISCV] Remove ForceTailAgnostic flag from vmv.s.x, vfmv.s.f and reductions..

LGTM, nice catch.

Tue, Jun 8, 1:58 AM · Restricted Project

Mon, Jun 7

frasercrmck committed rGae3f6de3a856: [InstCombine] Support negation of scalable-vector splats (authored by frasercrmck).
[InstCombine] Support negation of scalable-vector splats
Mon, Jun 7, 7:22 AM
frasercrmck closed D103801: [InstCombine] Support negation of scalable-vector splats.
Mon, Jun 7, 7:22 AM · Restricted Project
frasercrmck committed rGfd3b556958a9: [Constants] Extend support for scalable-vector splats (authored by frasercrmck).
[Constants] Extend support for scalable-vector splats
Mon, Jun 7, 6:46 AM
frasercrmck closed D103421: [Constants] Extend support for scalable-vector splats.
Mon, Jun 7, 6:46 AM · Restricted Project
frasercrmck updated the summary of D103421: [Constants] Extend support for scalable-vector splats.
Mon, Jun 7, 6:43 AM · Restricted Project
frasercrmck added inline comments to D103421: [Constants] Extend support for scalable-vector splats.
Mon, Jun 7, 4:03 AM · Restricted Project
frasercrmck requested review of D103801: [InstCombine] Support negation of scalable-vector splats.
Mon, Jun 7, 4:02 AM · Restricted Project
frasercrmck updated the diff for D103421: [Constants] Extend support for scalable-vector splats.
  • slice out change to dyn_castNegVal
  • update test with FIXME accordingly
Mon, Jun 7, 3:56 AM · Restricted Project
frasercrmck added inline comments to D103421: [Constants] Extend support for scalable-vector splats.
Mon, Jun 7, 2:30 AM · Restricted Project
frasercrmck updated the diff for D103421: [Constants] Extend support for scalable-vector splats.
  • rebase
  • add test fixme comment
  • normalize check order to fixed-length then (scalable) splat
  • consistently remove "this->" from these methods
Mon, Jun 7, 2:29 AM · Restricted Project
frasercrmck added inline comments to D103421: [Constants] Extend support for scalable-vector splats.
Mon, Jun 7, 2:17 AM · Restricted Project
frasercrmck added a comment to D103421: [Constants] Extend support for scalable-vector splats.

Ping.

Mon, Jun 7, 12:36 AM · Restricted Project

Fri, Jun 4

frasercrmck added a comment to D103510: [RISCV] Use vmv.v.[v|i] if we know COPY is under the same vl and vtype..

My browser's really chugging on this huge patch so my input has to be brief. Maybe we could hide the test changes for now?

Fri, Jun 4, 6:54 AM · Restricted Project
frasercrmck accepted D103299: [RISCV] Teach vsetvli insertion pass that operations on masks don't care about SEW/LMUL..

LGTM, thanks

Fri, Jun 4, 6:38 AM · Restricted Project
frasercrmck committed rGaec9cbbeb863: [SelectionDAG] Extend FoldConstantVectorArithmetic to SPLAT_VECTOR (authored by frasercrmck).
[SelectionDAG] Extend FoldConstantVectorArithmetic to SPLAT_VECTOR
Fri, Jun 4, 2:01 AM
frasercrmck closed D103246: [SelectionDAG] Extend FoldConstantVectorArithmetic to SPLAT_VECTOR.
Fri, Jun 4, 2:01 AM · Restricted Project

Thu, Jun 3

frasercrmck added a comment to D103603: [Sema][RISCV][SVE] Allow ?: to select Typedef BuiltinType in C.

Testcase for AArch64/SVE:

#include <arm_sve.h>

svint8_t a();
__SVInt8_t b();

svint8_t foo(int cond){
  return cond ? a(): b();
}
Thu, Jun 3, 3:37 AM · Restricted Project
frasercrmck committed rG8790e85255d0: [RISCV] Reserve an emergency spill slot for any RVV spills (authored by frasercrmck).
[RISCV] Reserve an emergency spill slot for any RVV spills
Thu, Jun 3, 2:52 AM
frasercrmck closed D103269: [RISCV] Reserve an emergency spill slot for any RVV spills.
Thu, Jun 3, 2:52 AM · Restricted Project
frasercrmck committed rG1de1887f5f18: [CodeGen] Fix a scalable-vector crash in VSELECT legalization (authored by frasercrmck).
[CodeGen] Fix a scalable-vector crash in VSELECT legalization
Thu, Jun 3, 2:33 AM
frasercrmck closed D103536: [CodeGen] Fix a scalable-vector crash in VSELECT legalization.
Thu, Jun 3, 2:33 AM · Restricted Project
frasercrmck added inline comments to D103299: [RISCV] Teach vsetvli insertion pass that operations on masks don't care about SEW/LMUL..
Thu, Jun 3, 2:18 AM · Restricted Project
frasercrmck updated the diff for D103269: [RISCV] Reserve an emergency spill slot for any RVV spills.
  • rebase
  • let the build bots have a go at it
Thu, Jun 3, 1:52 AM · Restricted Project
frasercrmck added inline comments to D103536: [CodeGen] Fix a scalable-vector crash in VSELECT legalization.
Thu, Jun 3, 1:49 AM · Restricted Project
frasercrmck updated the diff for D103536: [CodeGen] Fix a scalable-vector crash in VSELECT legalization.
  • rebase & add a fixme
Thu, Jun 3, 1:48 AM · Restricted Project
frasercrmck committed rG2dd20a31f27e: [ValueTypes] Fix scalable-vector changeExtendedVectorTypeToInteger (authored by frasercrmck).
[ValueTypes] Fix scalable-vector changeExtendedVectorTypeToInteger
Thu, Jun 3, 1:45 AM
frasercrmck closed D103534: [ValueTypes] Fix scalable-vector changeExtendedVectorTypeToInteger.
Thu, Jun 3, 1:45 AM · Restricted Project

Wed, Jun 2

frasercrmck requested review of D103536: [CodeGen] Fix a scalable-vector crash in VSELECT legalization.
Wed, Jun 2, 10:12 AM · Restricted Project
frasercrmck requested review of D103534: [ValueTypes] Fix scalable-vector changeExtendedVectorTypeToInteger.
Wed, Jun 2, 9:39 AM · Restricted Project
frasercrmck added inline comments to D103269: [RISCV] Reserve an emergency spill slot for any RVV spills.
Wed, Jun 2, 8:07 AM · Restricted Project