Page MenuHomePhabricator

asb (Alex Bradbury)
Director and Co-founder, lowRISC CIC

Projects

User does not belong to any projects.

User Details

User Since
Aug 6 2013, 5:31 AM (389 w, 3 d)

Recent Activity

Today

asb accepted D95150: [RISCV] Add B extension tests to make sure RV64 only instructions aren't accepted in RV32..

Thanks for extending the test coverage!

Fri, Jan 22, 10:58 AM · Restricted Project
asb accepted D95002: [RISCV] Update B extension version to 0.93..
Fri, Jan 22, 10:37 AM · Restricted Project, Restricted Project
asb accepted D94999: [RISCV] Add xperm.* instructions to Zbp extension..
Fri, Jan 22, 10:35 AM · Restricted Project
asb accepted D94944: [RISCV] Add support for rev8 and orc.b to Zbb..

LGTM, and I agree with the reasoning re preferring to point users towards zbb.

Fri, Jan 22, 10:33 AM · Restricted Project
asb accepted D94818: [RISCV] Add zext.h instruction to Zbb..
Fri, Jan 22, 10:26 AM · Restricted Project
asb accepted D94742: [RISCV] Move pack instructions to Zbp extension only..
Fri, Jan 22, 9:30 AM · Restricted Project
asb accepted D94736: [RISCV] Change zext.w to be an alias of add.uw rd, rs1, x0 instead of pack..

LGTM, added two minor notes.

Fri, Jan 22, 9:22 AM · Restricted Project
asb accepted D95090: [RISCV] Modify add.uw patterns to put the masked operand in rs1 to match 0.93 bitmanip spec..
Fri, Jan 22, 9:15 AM · Restricted Project
asb added inline comments to D94653: [RISCV] Rename Zbs instructions to start with just 'b' instead of 'sb' to match 0.93 bitmanip spec..
Fri, Jan 22, 9:12 AM · Restricted Project
asb accepted D94653: [RISCV] Rename Zbs instructions to start with just 'b' instead of 'sb' to match 0.93 bitmanip spec..

LGTM, going with the 0.94 deconflicted naming seems the only reasonable thing to do https://github.com/riscv/riscv-bitmanip/pull/102/commits/b962f8a04be570a93c3c4788425ee2e8a14e9c56

Fri, Jan 22, 9:10 AM · Restricted Project
asb accepted D94652: [RISCV] Move Shift Ones instructions from Zbb to Zbp to match 0.93 bitmanip spec..
Fri, Jan 22, 9:04 AM · Restricted Project

Yesterday

asb added a comment to D94652: [RISCV] Move Shift Ones instructions from Zbb to Zbp to match 0.93 bitmanip spec..

My hope is to get the 0.93 move into LLVM 12. Zbb is marked frozen in the 0.93 spec and does not include these. So I'd at least like them out of Zbb. Would it be better to just remove them until they have a home?

Mm yes, if Zbb is frozen and doesn't include them then I agree they need to go somewhere. Removing them sounds a bit drastic. If the release branch will be created in 6 days, how long can we delay the decision?

Thu, Jan 21, 7:58 AM · Restricted Project
asb accepted D94637: [RISCV] Add SH*ADD(.UW) instructions to Zba extension based on 0.93 bitmanip spec..

Not a blocker as I think this is just something that bitmanip has been less complete with vs the standard extensions, and you're just the last person to touch this, but we should probably be testing that rv32zba rejects rv64-only instructions and that rv64zba accepts the rv32 instructions. See rv64f-valid.s and rv32f-valid.s.

Thu, Jan 21, 7:27 AM · Restricted Project
asb accepted D94617: [RISCV] Add Zba feature and move add.uw and slli.uw to it..
Thu, Jan 21, 7:19 AM · Restricted Project, Restricted Project
asb accepted D94582: [RISCV] Rename mnemonics slliu.w->slli.uw and addu.w->add.uw to match 0.93 bitmanip spec..
Thu, Jan 21, 7:14 AM · Restricted Project, Restricted Project
asb accepted D94580: [RISCV] Swap encodings of max and minu to match 0.93 bitmanip spec..
Thu, Jan 21, 7:13 AM · Restricted Project
asb accepted D94577: [RISCV] Remove addiwu, addwu, subwu, subuw, clmulw, clmulrw, clmulhw to match 0.93 bitmanip spec..
Thu, Jan 21, 7:11 AM · Restricted Project
asb accepted D94568: [RISCV] Rename pcnt->cpop to match 0.93 bitmanip spec..

Straight forward change, LGTM. It seems there's nothing contentious in the patchset, so I'd expect the series can land basically all at once, to minimise the time we support a weird 0.92/0.93 hybrid.

Thu, Jan 21, 7:07 AM · Restricted Project
asb accepted D95106: [RISCV] Add isel patterns for SH*ADD(.UW).

These LGTM. This patch should presumably be marked dependent on D94637.

Thu, Jan 21, 5:55 AM · Restricted Project
asb added a comment to D95002: [RISCV] Update B extension version to 0.93..

I don't think any of the other patches in the stack update the comment at the top of RISCVInstrInfoB.td to say "version 0.92" rather than "version 0.93", and this is probably a reasonable patch to do it in.

Thu, Jan 21, 5:51 AM · Restricted Project, Restricted Project
asb added inline comments to D94931: [RISCV] Add attribute support for all supported extensions.
Thu, Jan 21, 5:24 AM · Restricted Project
asb added a comment to D94403: [RISCV] Implement new architecture extension macros.

@kito-cheng could you please confirm that this patch handles sub-extensions in the same way GCC does. i.e. -march=rv32izbb0p92 defines __riscv_zbb but NOT __riscv_b? That seems logical to me, as otherwise it would be cumbersome to check if the whole extension is supported rather than just a subset, but I just wanted to confirm.

Thu, Jan 21, 5:18 AM · Restricted Project

Thu, Jan 14

asb added a comment to D94589: [RISCV] Add intrinsics for vector AMO instructions.

Nitpick: this would be better titled "[RISCV] Add intrinsics for vector AMO instructions" - I was a little confused seeing the title come past :)

Thu, Jan 14, 6:35 AM · Restricted Project
asb added a comment to D94568: [RISCV] Rename pcnt->cpop to match 0.93 bitmanip spec..

RVB 0.93 is an awkward version to me, there is mnemonic conflict which is not resolved during release process since it's kind of too rush, the conflict one is bext in zbe and zbs...

However it's also a milestone for B-ext, since this version claim zba, zbb and zbc is frozen, maybe those 3 sub-ext. could be removed from the umbrella of -menable-experimental-extension ?

@asb What do you think about this?

Thu, Jan 14, 5:25 AM · Restricted Project
asb accepted D93168: [RISCV] Merge Utils library into MCTargetDesc.

I think this is a reasonable change. The pattern of having "Utils" is common to AArch64, AMDGPU, ARM, and RISCV but I agree with your point that it's not cleanly separated from MCTargetDesc, so there's little benefit.

Thu, Jan 14, 5:00 AM · Restricted Project

Dec 10 2020

asb updated subscribers of D91901: [RISCV] Move shift ComplexPatterns and custom isel to PatFrags with predicates.

Interesting. So to summarise my views on pros/cons

Dec 10 2020, 5:42 AM · Restricted Project
asb accepted D92008: [RISCV][LegalizeDAG] Expand SETO and SETUO comparisons. Teach LegalizeDAG to expand SETUO expansion when UNE isn't legal..

It's great to be able to drop those ugly patterns - nice cleanup. Thanks.

Dec 10 2020, 5:09 AM · Restricted Project
asb accepted D92793: [RISCV] Add (Proposed) Assembler Extend Pseudo-Instructions.

This needs a rebase, but otherwise looks good to me. Thanks Sam!

Dec 10 2020, 5:00 AM · Restricted Project
asb added a comment to D92715: [Clang][RISCV] Define RISC-V V builtin types.

Can we discuss this patch in tomorrows RISC-V meeting? @jrtc27 @kito-cheng @khchen @liaolucy

Dec 10 2020, 4:49 AM · Restricted Project

Nov 26 2020

asb added a comment to D83059: [RISCV] Use Generated Instruction Uncompresser.

I'm afraid this causes compile-time failures with 20030323-1.c, multi-ix.c, and pr53645.c.

Nov 26 2020, 8:22 AM · Restricted Project

Nov 13 2020

asb accepted D91414: [RISCV] Use a macro to simplify getTargetNodeName.

I think moving NODE_NAME_CASE(foo) to the left so it's aligned where the case statement would be (same as the equivalent in X86ISelLowering and AMDGPUISelLowering) would be slightly better. I wouldn't be opposed to using // clang-format off to stop clang-format from reformatting (though it seems more common in LLVM right now to just ignore clang-format's preference).

Nov 13 2020, 6:19 AM · Restricted Project

Nov 12 2020

asb added a reviewer for D90973: [RISCV] Remove RV32 HwMode. Use DefaultMode for RV32: kparzysz.

Ouch - I'm sorry to have contributed to the bloating of LLVM binaries!

Nov 12 2020, 5:57 AM · Restricted Project
asb accepted D91114: [RISCV] Don't include CodeGen layer files in MC layer.

This seems like a good cleanup to me - thanks.

Nov 12 2020, 5:50 AM · Restricted Project
asb added a comment to D91199: [RISCV] Remove traces of Glue from RISCVISD::SELECT_CC.

LGTM; from a quick look I can't see anything that would need that glue. Do you have an idea why it was added?

No. As far as I could tell it had been there but not used since the first patch that added RISCVISD::SELECT_CC.

Nov 12 2020, 5:45 AM · Restricted Project
asb added a comment to D90738: [RISCV] Support Zfh half-precision floating-point extension..

Historically for the RISC-V backend we've split patches into the MC layer changes and the codegen changes in order to make them easier to review. Do you think you'd be able to do the same here?

Nov 12 2020, 5:37 AM · Restricted Project

Oct 15 2020

asb accepted D89237: [RISCV] Do not grow the stack a second time when we need to realign the stack.

Thanks Roger, this looks good to me and doesn't seem to break anything in the GCC torture suite at least.

Oct 15 2020, 5:04 AM · Restricted Project
asb added a comment to D89288: [RISCV] Enable the use of the old sptbr name.

I think there's a little more to supporting older names than just the date of the spec. I agree that we don't expect anyone doing serious RISC-V work to be working to the 1.9 spec (unless there are cores taped out with that spec we don't know about), but in cases where there have been CSR renames it's often the case that people migrate to newer specification versions but don't always change the naming. There's an argument for the compiler complaining at them in this case, but there's also the user experience issue. It looks like the RISC-V compliance suite for instance still uses the sptbr name https://github.com/riscv/riscv-compliance/issues/107

Oct 15 2020, 5:02 AM · Restricted Project
asb accepted D86457: [compiler-rt][builtins][RISCV] Always include __mul[sd]i3 builtin definitions.

This LGTM. @lenary are you happy your comments are addressed?

Oct 15 2020, 4:49 AM · Restricted Project
asb accepted D83210: [RISCV][NFC] Add more tests for 32-bit constant materialization.

LGTM, thanks Luis.

Oct 15 2020, 3:33 AM · Restricted Project
asb accepted D89330: [RISCV] [TableGen] Modify RISCVCompressInstEmitter.cpp to use getAllDerivedDefinitions().

LGTM, thanks Paul.

Oct 15 2020, 3:18 AM · Restricted Project

Sep 17 2020

asb accepted D84414: [RISCV] Support Shadow Call Stack.

I think once @jrtc27 confirms all her issues are addressed this is good to land.

Sep 17 2020, 5:49 AM · Restricted Project, Restricted Project

Sep 3 2020

asb accepted D85366: [RISCV] Do not mandate scheduling for CSR instructions.

LGTM, thanks.

Sep 3 2020, 4:55 AM · Restricted Project
asb added a comment to D86836: Support a list of CostPerUse values.

It looks like others are reviewing the implementation details, but I just wanted to chime in to say that this looks like a useful feature for the RISC-V backend too. Currently we set CostPerUse for some registers in order to tweak register allocation in a way that benefits codegen when the compressed instruction set extension is enabled. Although the impact is _very_ minimal, this slightly harms targets that don't support the compressed instruction set (admittedly, this isn't really true of any shipping RISC-V processor so not a big concern). It would be good to use this vary the CostPerUse depending on target RISC-V features.

Sep 3 2020, 4:49 AM · Restricted Project
asb accepted D87069: [NFC][RISCV] Simplify pass arg of RISCVMergeBaseOffsetOpt.
Sep 3 2020, 4:44 AM · Restricted Project

Aug 21 2020

asb accepted D86286: [RISCV] Fix inaccurate annotations on PseudoBRIND.
Aug 21 2020, 2:30 AM · Restricted Project
asb added a comment to D77152: [SelectionDAG] Better legalization for FSHL and FSHR.

No objections accepting from the RISC-V side. I don't have a set of execution tests for the fshl/fshr changes to validate them, but I didn't see anything obviously wrong. The changes primarily affect the (still experimental) bitmanip extension also, so the bar for review is somewhat lower. The change does seem to improve srliw lowering in a few places in the GCC torture suite as well (in a way that is both correct and beneficial).

Aug 21 2020, 2:27 AM · Restricted Project

Aug 20 2020

asb committed rG1ecf120246e7: [index-while-building] Fix build with -DBUILD_SHARED_LIBS=True (authored by asb).
[index-while-building] Fix build with -DBUILD_SHARED_LIBS=True
Aug 20 2020, 7:14 AM
asb added a comment to D85366: [RISCV] Do not mandate scheduling for CSR instructions.

The description of isNotDuplicable in MachineInstr.h is:

/// Return true if this instruction cannot be safely duplicated.
/// For example, if the instruction has a unique labels attached
/// to it, duplicating it would cause multiple definition errors.
bool isNotDuplicable(QueryType Type = AnyInBundle) const {
  return hasProperty(MCID::NotDuplicable, Type);
}

I don't think this obviously applies to CSRs, and the property doesn't seem to be applied to instructions that modify control registers for other in-tree targets.

Aug 20 2020, 2:28 AM · Restricted Project

Aug 6 2020

asb accepted D85015: [RISCV] Enable MCCodeEmitter instruction predicate verifier.

Thanks for spotting this one - LGTM.

Aug 6 2020, 6:43 AM · Restricted Project
asb added a comment to D84833: Implement indirect branch generation in position independent code for the RISC-V target.

@jrtc27 are you happy with this patch in its current form or are there outstanding issues to be addressed?

Aug 6 2020, 6:28 AM · Restricted Project

Jul 30 2020

asb added a comment to D84833: Implement indirect branch generation in position independent code for the RISC-V target.

@jrtc27 are you happy with this patch now? Thanks for your review on this and thanks @msizanoen1 for providing this fix.

Jul 30 2020, 5:37 AM · Restricted Project

Jul 21 2020

asb added a comment to D79870: [RISCV] Add matching of codegen patterns to RISCV Bit Manipulation Zbb asm instructions.

It's a shame this just missed the creation of the llvm 11.0 branch, do we think it's worth trying to get this backported since it only just missed?

Jul 21 2020, 6:51 AM · Restricted Project

Jul 16 2020

asb accepted D71124: [RISCV] support clang driver to select cpu.

LGTM, thanks!

Jul 16 2020, 7:06 AM · Restricted Project, Restricted Project

Jul 14 2020

asb added a comment to D83059: [RISCV] Use Generated Instruction Uncompresser.

I'm getting a test failure on rv64-relax-all.s with this patch?

Jul 14 2020, 11:04 PM · Restricted Project
asb added a comment to D77443: [RISCV] Fix RISCVInstrInfo::getInstSizeInBytes for atomics pseudos.

Hi @jrtc27 - it would be nice to get this in before LLVM 11 branches. Will you have time to add the extra comment and commit today?

Jul 14 2020, 10:56 PM · Restricted Project
asb added a comment to D65802: [DAGCombiner] Rebuild (setcc x, y, ==) from (xor (xor x, y), 1).

This looks good to land IMHO. It's a nice improvement for RISC-V, and it seems to have been sufficiently reviewed by people active with the DAGCombiner. Thanks Roger.

Jul 14 2020, 10:53 PM · Restricted Project
asb accepted D83819: [RISCV] Add error checking for extensions missing separating underscores.

LGTM, thanks!

Jul 14 2020, 10:28 PM · Restricted Project
asb added a comment to D71124: [RISCV] support clang driver to select cpu.

I've added some suggestions to clarify the code comments. I think before landing it would be good to address the crash Sam pointed out for an invalid -march, but otherwise I think this looks good to me (at least, it seems worth landing this and if further issues crop up we can fix them and request them for merging into the LLVM 11 branch).

Jul 14 2020, 10:14 PM · Restricted Project, Restricted Project
asb added a comment to D83754: [Attributor] Unittest for Attributor.

This broke the build for -DBUILD_SHARED_LIBS=True. I've committed rG5282a6186c which fixes it for me.

Jul 14 2020, 9:07 PM · Restricted Project
asb committed rG5282a6186cfb: [Attributor] Fix build of unittest with DBUILD_SHARED_LIBS=True (authored by asb).
[Attributor] Fix build of unittest with DBUILD_SHARED_LIBS=True
Jul 14 2020, 9:06 PM

Jul 9 2020

asb accepted D77030: [RISCV] refactor FeatureRVCHints to make ProcessorModel more intuitive.

Let's go for it! Thanks.

Jul 9 2020, 10:50 AM · Restricted Project
asb added a comment to D81583: Update SystemZ ABI to handle C++20 [[no_unique_address]] attribute.

This LGTM from a RISC-V perspective. I'll likely follow up with a RISC-V test case similar to the SystemZ one post-commit, but given this is really fixing a cross-platform ABI issue this seems non-urgent. Thanks for spotting and addressing this issue.

Jul 9 2020, 4:40 AM · Restricted Project
asb accepted D82988: [RISCV] Avoid Splitting MBB in RISCVExpandPseudo.

Got it, thanks. In that case LGTM and please tweak the commit message (the last paragraph specifically) so it's clear that the two changes are interlinked.

Jul 9 2020, 3:03 AM · Restricted Project

Jul 8 2020

asb accepted D81805: [RISCV] Fix isStoreToStackSlot.

This looks good to me, good catch. I do see a codegen change on regstack-1.c from the GCC Torture Suite, so it might be worth having a quick look to see if it's easy to make a test case based on that.

Jul 8 2020, 11:45 PM · Restricted Project
asb requested changes to D82988: [RISCV] Avoid Splitting MBB in RISCVExpandPseudo.

This is a nice simplification, thanks. My only request before committing is to split out the RISCVTargetMachine to a separate pass, as that is logically distinct.

Jul 8 2020, 10:37 PM · Restricted Project
asb accepted D77443: [RISCV] Fix RISCVInstrInfo::getInstSizeInBytes for atomics pseudos.

LGTM, +1 on adding a comment to the expansion functions noting the need to update getInstSizeInBytes. Thanks!

Jul 8 2020, 10:25 PM · Restricted Project
asb added a comment to D80802: [RISCV] Upgrade RVV MC to v0.9..

I've gone through and can't see any obvious issues. I defer to one of the RISC-V Vector extension usual suspects for giving a LGTM on the detail of the altered instructions etc. Once we have that, this looks good to land IMHO.

Jul 8 2020, 10:19 PM · Restricted Project, Restricted Project
asb added inline comments to D83159: [RISCV][test] Add a new codegen test of add-mul transform.
Jul 8 2020, 10:08 PM · Restricted Project
asb added a comment to D71124: [RISCV] support clang driver to select cpu.

This has been hanging around for a while, but I think we'd basically agreed this is the right logic. The comments have ended up referring to flags that don't exist on Clang making it a little hard to follow, and I've added a request to slightly expand testing. If you make those cleanups I think it should be ready for a final review and merge.

Jul 8 2020, 9:52 PM · Restricted Project, Restricted Project

Jun 30 2020

asb accepted D82913: [RISCV] Add mcountinhibit CSR.

Looks good to me, thanks for catching this.

Jun 30 2020, 9:39 PM · Restricted Project

Jun 25 2020

asb added inline comments to D69987: [RISCV] Assemble/Disassemble v-ext instructions..
Jun 25 2020, 6:20 AM · Restricted Project, Restricted Project
asb accepted D69987: [RISCV] Assemble/Disassemble v-ext instructions..

The patch as it stands now LGTM and I think it can be committed. Is there any objection remaining?

Any further comments @simoncook @asb?

Jun 25 2020, 5:52 AM · Restricted Project, Restricted Project

Jun 18 2020

asb accepted D80526: [RISCV64] emit correct lib call for fp(double) to ui/si.

Thanks, this looks good to me. I wasn't aware of MakeLibCallOptions and IsSoften - I think I've wanted something like that before.

Jun 18 2020, 5:57 AM · Restricted Project

Jun 12 2020

asb committed rG3dcfd482cb17: [CodeGen] Increase applicability of ffine-grained-bitfield-accesses for targets… (authored by asb).
[CodeGen] Increase applicability of ffine-grained-bitfield-accesses for targets…
Jun 12 2020, 2:41 AM
asb closed D79155: [CodeGen] Increase applicability of ffine-grained-bitfield-accesses for targets with limited native integer widths.
Jun 12 2020, 2:41 AM · Restricted Project
asb added a comment to D79155: [CodeGen] Increase applicability of ffine-grained-bitfield-accesses for targets with limited native integer widths.

Please add a comment explaining what OffsetInRecord means; then LGTM.

Jun 12 2020, 1:35 AM · Restricted Project

Jun 10 2020

asb added a comment to D79155: [CodeGen] Increase applicability of ffine-grained-bitfield-accesses for targets with limited native integer widths.

Ping on this. The patch still applies cleanly against current HEAD.

Jun 10 2020, 11:12 PM · Restricted Project
asb committed rGd9bc8bd54a70: [RISCV] Make visibility of overridden methods in RISCVISelLowering match the… (authored by asb).
[RISCV] Make visibility of overridden methods in RISCVISelLowering match the…
Jun 10 2020, 1:36 AM
asb closed D79928: [RISCV] Make visibility of overridden methods in RISCVISelLowering match the parent.
Jun 10 2020, 1:36 AM · Restricted Project

Jun 9 2020

asb added reviewers for D81348: [compiler-rt][builtins] Add tests for atomic builtins support functions: jfb, jyknight.

I think there's value in adding these tests (I note libatomic in GCC has a similar set of simple functionality tests), and this seems a sensible way to do it.

Jun 9 2020, 2:42 AM · Restricted Project
asb added a comment to D81391: [RISC-V] Do not crash when using -ftrapping-math.

Disclaimer: I haven't swotted up on these constrained intrinsics to review the proposed lowering yet, but added a quick note on the setOperationAction calls. I agree with Luis the test cases would probably be easier to read if using the hard float ABI. I think you're lacking any test coverage for f32 as well.

Jun 9 2020, 1:03 AM · Restricted Project

May 28 2020

asb added a comment to D79870: [RISCV] Add matching of codegen patterns to RISCV Bit Manipulation Zbb asm instructions.

Sorry for the delay on this - the lockdown situation is really hurting my review time, though it looks like my childcare situation will improve from the week after next.

May 28 2020, 7:34 AM · Restricted Project

May 19 2020

asb added inline comments to D79141: [RISCV] Better Split Stack Pointer Adjustment for RVC.
May 19 2020, 1:03 AM · Restricted Project

May 14 2020

asb accepted D79770: [RISCV] Fix passing two floating-point values in complex separately by two GPRs on RV64 .

Good catch, thanks for the fix! The logic was incorrectly written assuming isFloatingType would return false for complex values which is of course incorrect.

May 14 2020, 3:10 AM · Restricted Project
asb added a comment to D78545: [RISCV] Make CanLowerReturn protected for downstream maintenance.

Please check D79928 which cleans up the visibility of all of these overridden methods.

May 14 2020, 2:38 AM · Restricted Project
asb created D79928: [RISCV] Make visibility of overridden methods in RISCVISelLowering match the parent.
May 14 2020, 2:38 AM · Restricted Project
asb added a comment to D79492: [RISCV] Improve constant materialization.

I'm in favour of merging a patch that is functionally correct and makes incremental improvements. I'd rather avoid any regression in codesize though if possible. How feasible would it to be to avoid that case? It may be worthwhile leaving some of the materialisation improvements for a follow-up patch in order to land the clear wins now.

May 14 2020, 2:05 AM · Restricted Project

May 12 2020

asb accepted D79635: [RISCV] Split the pseudo instruction splitting pass.

Thanks, this looks good to me. PreSched2 is logically a better place for the standard pseudo expansions, though I'm not seeing any codegen changes at all with e.g. the GCC torture suite. But I'm happy to land this as-is.

May 12 2020, 12:30 AM · Restricted Project
asb added a comment to D79690: [RISCV] Fold ADDIs into load/stores with nonzero offsets.

Could you rebase please? This isn't applying cleanly for me on current master.

May 12 2020, 12:30 AM · Restricted Project

May 11 2020

asb added a comment to D78545: [RISCV] Make CanLowerReturn protected for downstream maintenance.

Looks like this landed while I was composing the below:

May 11 2020, 11:25 PM · Restricted Project

May 7 2020

asb added a comment to D67348: [RISCV] Add codegen pattern matching for bit manipulation assembly instructions..

Hi Paolo. I'm sorry this has been left hanging for some time. On the one hand, with this being an experimental feature and purely additive the bar for merging is slightly lower than e.g. a rewrite of all our existing codegen patterns (which could cause new regressions). On the other hand, this pre-commit review is realistically going to be the time when the codegen patterns and associated tests get most scrutiny and it would be a shame to skip that.

May 7 2020, 6:12 AM · Restricted Project

May 6 2020

asb added a comment to D79492: [RISCV] Improve constant materialization.

Thanks, this is a good improvement. Two thoughts, mainly for discussion rather than blocking issues:

May 6 2020, 11:59 PM · Restricted Project
asb requested changes to D79141: [RISCV] Better Split Stack Pointer Adjustment for RVC.

Thanks Sam. I left a few minor comments inline. From looking at the diff in compiler output on the GCC torture suite, I do see a few cases where there is a potential regression. e.g. loop-ivopts-2.c, which represents a case where the stack adjustment is large, but doesn't overflow the 12-bit immediate fields of addi and s[d|w]. The prologue and epilogue are both two instructions and become three instructions with this patch (although it is a code size win).

May 6 2020, 11:59 PM · Restricted Project
asb added a comment to D79521: [RISCV] Add SiFive's interrupt modes.

Just to understand the current status with respect to GCC, am I right that support for these attributes is not in upstream GCC? @kito-cheng is there any plan to upstream?

May 6 2020, 11:15 PM · Restricted Project
asb accepted D79523: [RISCV] Support Constant Pools in Load/Store Peephole.

LGTM, thanks. Left an inline comment pointing out a TODO that can now be removed thanks to this.

May 6 2020, 10:42 PM · Restricted Project

May 5 2020

asb accepted D78764: [RISCV] Update debug scratch register names.

LGTM, thanks. Left one tiny nit inline.

May 5 2020, 8:03 AM · Restricted Project

Apr 30 2020

asb added inline comments to D78905: [RISCV][NFC] Tests for indirect float conversion.
Apr 30 2020, 4:43 AM · Restricted Project
asb added a comment to D78907: [RISCV][NFC] Add tests for checking isnan patterns.
In D78907#2012344, @asb wrote:

LGTM. Minor nit: using %0 and %2 as the only two named values seems odd though? Most other tests in test/CodeGen/RISCV use a/b/... for arguments and %0/%1/%2/... in the body.

I generated the testcases in Clang (actually using godbolt, but it's doing the same IR lowering as clang HEAD is), which is why the arguments don't have names. I can re-introduce them if that's easier.

Apr 30 2020, 4:43 AM · Restricted Project
asb accepted D78907: [RISCV][NFC] Add tests for checking isnan patterns.

LGTM. Minor nit: using %0 and %2 as the only two named values seems odd though? Most other tests in test/CodeGen/RISCV use a/b/... for arguments and %0/%1/%2/... in the body.

Apr 30 2020, 3:37 AM · Restricted Project
asb accepted D78908: [RISCV] Add patterns for checking isnan.

LGTM, thanks!

Apr 30 2020, 3:37 AM · Restricted Project