User Details
- User Since
- May 10 2016, 6:42 AM (256 w, 4 d)
Wed, Apr 7
LGTM. Thanks @HsiangKai.
LGTM
Thu, Apr 1
My question is how to count the scalar incoming argument with this frame layout,
because scalar incoming arguments cross RVV previous local variables frame objects.
In the current frame(callee), we don't know the RVV previous local variables count.
So we can't get the real stack offset because callee not know the caller vector local variables size.
One could argue that this introduces coupling between the scalar register bank and the vector register bank. But I presume the simpler scalar load makes up for that loss of decoupling.
Wed, Mar 31
I was under the impression we didn't want to use class-member access syntax for vector tuples (see https://github.com/riscv/rvv-intrinsic-doc/issues/17#issuecomment-628998077 ) so we don't need a record type, do we?
This is just a suggestion, feel free to ignore: a sequence of Ts was easy to parse for the prototype but may be want to consider something like T3v rather than TTTv. I think it would simplify the TString tblgen class and those std::string(N, 'T').
Tue, Mar 30
Hi @SuH wrote:
I have a question about access incoming args.
Mon, Mar 29
ChangeLog:
- Rebase
ChangeLog:
- Describe a bit better the purpose of the test.
Fri, Mar 26
Tue, Mar 23
I have internally validated that the last change (Diff 331637, not Diff 332358 that only adds a comment) is equivalent to the previous change (Diff 331546).
Mon, Mar 22
ChangeLog:
- Add comment explaining why we add the alignment of the stack as RVV padding when CSR are not aligned to 8 bytes.
ChangeLog:
- Clarify the purpose of the change.
- Adjust syntax of the IR input.
Fri, Mar 19
Thu, Mar 18
ChangeLog:
- Fold the padding into the main stack size enlargement: adjust CSR offsets accordingly, as suggested by @StephenFan
- Add {get,set}RVVPadding to RISCVMachineFunctionInfo to materialise the extra padding required by RVV.
I feel this might a bit complicated to grasp from my attempt of explanation in the summary, so this is how the stack should look like for wrong-stack-slot-rv32.mir assuming VLENB=128.
ChangeLog:
- Add extra padding so we can adjust RVV objects within the stack correctly
- Round the offset to the RVV objects to 8 bytes (when not using fp).
- Move computation of CalleeSavedStackSize from processFunctionBeforeFrameIndicesReplaced to processFunctionBeforeFrameFinalized because we need that size before we emit the prologue.
ChangeLog:
- Simplify the MIR input
- Split test in two for RV32 and RV64
Wed, Mar 17
Hum, this is not right. I just realized that we may need some extra padding between the end of the scalar part and the RVV stack slots. That padding hasn't been accounted anywhere.
Mar 10 2021
Overall LGTM. Thanks @khchen!
Mar 9 2021
Hi. We did this out of an abundance of caution. We wanted to be sure after the vsetvli were emitted, we didn't attempt to use the SEW operand (either by accident or intentionally).
Except for the minor comments above, LGTM.
Mar 1 2021
Yes, I referenced this out-of-reach-emergency-slot.mir test. But with a stack object that has a out-of-range size. And I think my use-emergency-spill-slot.mir test and out-of-reach-emergency-slot.mir test are a little repeated. So should I keep my use-emergency-spill-slot.mir test or delete it?
Isn't this very similar to https://github.com/llvm/llvm-project/blob/main/llvm/test/CodeGen/RISCV/out-of-reach-emergency-slot.mir except for a missing SD?
Can you add some tests for the cases with overaligned stuff and variable sized + rvv?
Feb 23 2021
Feb 20 2021
Hi @StephenFan. I wonder if we want to do this only when we index via sp.
Feb 17 2021
Thanks @HsiangKai. LGTM.
Feb 16 2021
I'm happy with the patch as it stands now. At least it gives us a baseline to improve if that is needed.
Hi Sander, one question OptionalVFCandidates::addMaxVF combines using the maximum but AFAICT it is not used in a loop. Maybe you plan to "update" the corresponding VF values (scalable and/or fixed) in other places (not shown in the current patch, yet)?
Feb 5 2021
Feb 2 2021
Feb 1 2021
I understand it is not a win to apply a similar scheme to the base RISCVInstrInfo.td and perhaps F's and D's because we do have an offset available there. Is my understanding correct?
I admit that I initially wasn't convinced by the idea of ComplexPattern including both the matching logic (which is only return true) and a bit of selection logic (selectImm) in the input DAG. But on a second thought this allows the two logics be kept together.
Jan 29 2021
Hi Kai, are we OK with having a test that goes from IR to assembly in the Transforms component?
Jan 27 2021
LGTM. Thanks @craig.topper
Jan 25 2021
We're multiplying by 3 the _VF pseudos but to be honest I don't see any better alternative.
Jan 24 2021
Jan 23 2021
Jan 19 2021
Ping.
Jan 18 2021
This seems a very sensible approach.
Jan 15 2021
Jan 12 2021
I prefer this approach. Computing the address is a bit more involved but doesn't need an extra memory access.
Jan 11 2021
Hi Serge,
Jan 10 2021
This is a very nice cleanup. Thanks @craig.topper!
Jan 9 2021
ChangeLog:
- Largely simplify the testcase by making a call that uses all the eligible registers forcing the RegisterScavenger to use the emergency spill.
ChangeLog:
- Rebase patch prior committing
Jan 6 2021
Hi Serge, would it make sense to use a Pseudo for those specific cases with a custom inserter? (usesCustomInserter = 1 in the tablegen definition of the Pseudo)
Dec 17 2020
For instructions that use a uimm5 immediate, this change only affects
constants >= 128 for i8 or >= 32768 for i16. Constants that large
already wouldn't have been eligible for uimm5 and would need to use a
scalar register.
Dec 16 2020
Dec 14 2020
Agreed, these instructions and their intrinsics seems pretty uncontroversial. LGTM.
Also LGTM once Craig's comments are addressed. Thanks a lot Kai!
Dec 11 2020
This looks good to me. Thanks a lot @craig.topper
Dec 10 2020
Dec 4 2020
As a first step to remove the most obviously redundant vsetvli cases, this LGTM.
I don't know if we're going to have an answer for the masked/unmasked soon. I think we might just use earlyclobber in the initial patches and suffer the bad register allocation so we can make forward progress.
Dec 1 2020
Hi @simoll: a quick question regarding vp.load/vp.store/vp.gather/vp.scatter. Does the current definition of VPred allow for something similar to the !nontemporal metadata of regular load/store instructions? I don't see any explicit mention to that but maybe it is already possible using metadata or some other annotation?
Nov 30 2020
Hi all,
Nov 24 2020
Hi Andrey, I can confirm this fixes the build for RISC-V.
Nov 18 2020
Hi Todd,
Hi Todd,
Random number won't work, because the goal of the library registering is to catch several libraries in a single process. So a process should have deterministic file name (or environment variable name).
Nov 17 2020
[Drive by] What about appending the process id or a random number to the file name, or both?
Hi Todd,
Hi, one question about this change: