- User Since
- Dec 27 2017, 10:26 AM (108 w, 6 d)
Tue, Jan 21
FYI. Needs rebase to pick up https://reviews.llvm.org/D72649. Thanks.
Mon, Jan 20
Thu, Jan 16
Wed, Jan 15
Thanks for review and accepting.
Tue, Jan 14
Mon, Jan 13
Tue, Jan 7
Mon, Jan 6
@jhibbits: are you OK with this now? Most of the other reviewers are OK with this now. Thanks.
Sun, Jan 5
Yes, in theory, it might affect scheduling and other opts as well.
Can we please prove it by providing a test case that has changes due to this patch. Thanks.
Fri, Jan 3
Rebase to show changes related to this change only.
Thu, Jan 2
Yes, it would be better if you can run update_llc_test_checks.py without this patch first, commit it, rebase, then run update_llc_test_checks.py with this patch again.
Tue, Dec 31
Handle fp128 and fp16.
clang-tidy: fail. Please fix clang-tidy findings.
clang-tidy failure fixed in https://reviews.llvm.org/rGfcbf05bbdccc8a32f6a80316ea1c13be7e7eeae2.
Mon, Dec 30
I checked all references to isPredicable in codebase:
- Instr doc generation uses the bit (this is non-functional)
- CodeGen tablegen itself uses it
- Both TargetInstrInfo::isUnpredicatedTerminator and TargetInstrInfo::PredicateInstruction references the method, but PPC overrides them and doesn't use it.
- MachineInstr::findFirstPredOperandIdx references it, but only ARM and AMDGPU invokes it
- Implicit null check and machine sink have referenced it. However, this patch won't touch instrs may load or store, so result of expression would never change
- Other references are for other platforms
Looks like to me that Vector Compare in SPE also has different CR bit semantics .
So this patch will only handle floating point part for SPE? If so, maybe we should make it explicit in title.
Dec 27 2019
If this is a NFC change, please add [NFC] in summary. Or else, please add test. Thanks.
Dec 24 2019
@Jim Are you intend to block this by requesting changes? If so, can you have a look again? Thanks.
Dec 23 2019
Dec 16 2019
Dec 13 2019
@ZhangKang Can we get some performance data for this change. Thanks.
Dec 12 2019
Dec 9 2019
Rename to _rec instead.
Thanks. I will commit it for you.
Hi all, thanks for the review and discussion.
I'd like to make a final decision about commit or abandon, I don't see any point of leaving this longer.
@lkail When committing code for others, please follow the format described in Developer Policy. Thanks.
Dec 5 2019
Dec 3 2019
Thanks @jhibbits !
Dec 2 2019
Rename RLWINM(I)_recordbm to RLWINM(I)bm_record
Rename record instructions to use _record suffix instead of o
Rename isDot to isRecordForm
To make things more obvious, we could replace isDOT with isRecordForm, the 'o' suffix with _record (moving the numeric suffix, where present, to before the underscore). What do you think of that?
Furthermore, the flag we set in the multiclasses is called isDOT and there is no clear relationship between this "dot" and the letter r. All that said, I will not block this if the majority is in agreement (or indifferent). But let's get some more eyes on this.
Rebased to resolve conflict.