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 (284 w, 21 h)

Recent Activity

Mon, Jan 14

asb added a comment to D53235: [RISCV] Add RV64F codegen support.

Ah, I think what happens is that we should not emit such patterns without the presence of the F extension: these patterns are ultimately selected as instructions in F. Does this make sense?

Mon, Jan 14, 8:40 AM

Sat, Jan 12

asb updated the diff for D53237: [RISCV] Implement RV64D codegen.

Patch refresh, no changes. Ping?

Sat, Jan 12, 11:35 PM
asb updated the diff for D53235: [RISCV] Add RV64F codegen support.

Rebased (no meaningful changes).

Sat, Jan 12, 12:56 PM

Fri, Jan 11

asb committed rL350993: [RISCV] Introduce codegen patterns for RV64M-only instructions.
[RISCV] Introduce codegen patterns for RV64M-only instructions
Fri, Jan 11, 11:47 PM
asb closed D53230: [RISCV] Introduce codegen patterns for RV64M-only instructions.
Fri, Jan 11, 11:47 PM
asb committed rL350992: [RISCV] Add patterns for RV64I SLLW/SRLW/SRAW instructions.
[RISCV] Add patterns for RV64I SLLW/SRLW/SRAW instructions
Fri, Jan 11, 11:36 PM
asb closed D56264: [RISCV] Add patterns for RV64I SLLW/SRLW/SRAW instructions.
Fri, Jan 11, 11:36 PM
asb retitled D53233: [RISCV] Add codegen support for RV64A from [RISCV] Add codegen support RV64A to [RISCV] Add codegen support for RV64A.
Fri, Jan 11, 2:34 PM
asb updated the diff for D53233: [RISCV] Add codegen support for RV64A.

Updated. Now handles cmpxchg, supporting all RV64A. The code changes are actually pretty minimal so I hope this is fairly straight-forward to review.

Fri, Jan 11, 2:34 PM
asb edited reviewers for D56264: [RISCV] Add patterns for RV64I SLLW/SRLW/SRAW instructions, added: efriedma; removed: eli.friedman.
Fri, Jan 11, 2:22 PM
asb updated the diff for D53230: [RISCV] Introduce codegen patterns for RV64M-only instructions.

Thanks for the review @efriedma. Updated to add the suggested regression test.

Fri, Jan 11, 2:21 PM
asb committed rL350962: [RISCV][NFC] Add CHECK lines for atomic operations on RV64I.
[RISCV][NFC] Add CHECK lines for atomic operations on RV64I
Fri, Jan 11, 11:50 AM
asb updated the diff for D53230: [RISCV] Introduce codegen patterns for RV64M-only instructions.

Many thanks to @efriedma for catching the issue with one of the sdiv patterns. I've removed that pattern, and added similar logic to that used for udiv/urem so that anyext is converted to sext if this is likely to result in one of the *w instructions being selected (and thus avoiding unnecessary sext/zext of the input operands).

Fri, Jan 11, 7:41 AM

Thu, Jan 10

asb added a comment to D47755: [RISCV] Insert R_RISCV_ALIGN relocation type and Nops for code alignment when linker relaxation enabled.

Hi Alex, I have added a constant pool test case at the end of align.s to verify the padding behavior for constant pool. Once D45961 landing and changing the behavior, we should be able to aware. It seems that GCC will insert Nops for padding constant pool. Could you illustrate more about "use .align with a value to fill with"? I could test on GCC.

Thu, Jan 10, 8:48 AM
asb added a comment to D46677: [RISCV] Add R_RISCV_RELAX relocation to all possible relax candidates..

I think we've landed enough pre-requisites that this this is good to go in, but it introduces failures to option-pushpop.s and option-relax.s. Can you take a look please?

Thu, Jan 10, 8:37 AM
asb added a comment to D55560: [RISCV] Attach VK_RISCV_CALL to symbols upon creation.

I know this isn't a behaviour change vs the previous bare_symbol, but I note that GNU as currently accepts call with constants, e.g. call 1234. Should we be doing the same?

Thu, Jan 10, 7:45 AM
asb committed rL350831: [RISCV][MC] Add support for evaluating constant symbols as immediates.
[RISCV][MC] Add support for evaluating constant symbols as immediates
Thu, Jan 10, 7:37 AM
asb closed D52298: [RISCV][MC] Add support for evaluating constant symbols as immediates.
Thu, Jan 10, 7:37 AM
asb added inline comments to D53230: [RISCV] Introduce codegen patterns for RV64M-only instructions.
Thu, Jan 10, 7:31 AM
asb updated the diff for D56264: [RISCV] Add patterns for RV64I SLLW/SRLW/SRAW instructions.

Updated to address review comments from Eli (thanks!).

Thu, Jan 10, 6:35 AM
asb updated the diff for D56264: [RISCV] Add patterns for RV64I SLLW/SRLW/SRAW instructions.

Previous patch was the pre-clang-format version. This update fixes that!

Thu, Jan 10, 6:35 AM

Thu, Jan 3

asb updated the diff for D56264: [RISCV] Add patterns for RV64I SLLW/SRLW/SRAW instructions.

Updated to improve pattern definitions. The first patterns were over-constrained, as they would only match shifts where the shift amount operand matched the zexti32 pattern. I introduce the shiftwamt PatFrags so it will match a zexti32 shift amount (and hence avoid generating the unnecessary zero-extend), or a shift amount that isn't zero-extended.

Thu, Jan 3, 11:17 AM
asb removed a parent revision for D53237: [RISCV] Implement RV64D codegen: D53235: [RISCV] Add RV64F codegen support.
Thu, Jan 3, 8:27 AM
asb removed a child revision for D53235: [RISCV] Add RV64F codegen support: D53237: [RISCV] Implement RV64D codegen.
Thu, Jan 3, 8:27 AM
asb removed a parent revision for D53230: [RISCV] Introduce codegen patterns for RV64M-only instructions: D52977: [RISCV] Introduce codegen patterns for instructions introduced in RV64I.
Thu, Jan 3, 8:26 AM
asb removed a child revision for D52977: [RISCV] Introduce codegen patterns for instructions introduced in RV64I: D53230: [RISCV] Introduce codegen patterns for RV64M-only instructions.
Thu, Jan 3, 8:26 AM
asb removed a parent revision for D53235: [RISCV] Add RV64F codegen support: D56264: [RISCV] Add patterns for RV64I SLLW/SRLW/SRAW instructions.
Thu, Jan 3, 8:26 AM
asb removed a child revision for D56264: [RISCV] Add patterns for RV64I SLLW/SRLW/SRAW instructions: D53235: [RISCV] Add RV64F codegen support.
Thu, Jan 3, 8:26 AM
asb updated the diff for D53237: [RISCV] Implement RV64D codegen.

Refreshed patch.

Thu, Jan 3, 8:17 AM
asb updated the diff for D54574: [SelectionDAG] Support promotion of the FPOWI integer operand.

Refreshing patch, no functional changes.

Thu, Jan 3, 8:09 AM
asb removed a child revision for D52977: [RISCV] Introduce codegen patterns for instructions introduced in RV64I: D53235: [RISCV] Add RV64F codegen support.
Thu, Jan 3, 7:36 AM
asb edited parent revisions for D53235: [RISCV] Add RV64F codegen support, added: 2; removed: 1.
Thu, Jan 3, 7:36 AM
asb added a child revision for D56264: [RISCV] Add patterns for RV64I SLLW/SRLW/SRAW instructions: D53235: [RISCV] Add RV64F codegen support.
Thu, Jan 3, 7:36 AM
asb added a child revision for D53230: [RISCV] Introduce codegen patterns for RV64M-only instructions: D53235: [RISCV] Add RV64F codegen support.
Thu, Jan 3, 7:36 AM
asb updated the diff for D53235: [RISCV] Add RV64F codegen support.

Rebased on top of D56264 and D53230. Although this patch could be written to apply without those changes, given all patches modify the dag combines it's simpler to keep them linear.

Thu, Jan 3, 7:35 AM
asb abandoned D52299: [RISCV][MC] Accept %lo and %pcrel_lo on operands to li.
Thu, Jan 3, 6:47 AM
asb added a comment to D52299: [RISCV][MC] Accept %lo and %pcrel_lo on operands to li.

Closed by rL350321 (sorry, forgot to mention this review in the commit message).

Thu, Jan 3, 6:47 AM
asb committed rL350321: [RISCV][MC] Accept %lo and %pcrel_lo on operands to li.
[RISCV][MC] Accept %lo and %pcrel_lo on operands to li
Thu, Jan 3, 6:45 AM
asb removed a parent revision for D52299: [RISCV][MC] Accept %lo and %pcrel_lo on operands to li: D52298: [RISCV][MC] Add support for evaluating constant symbols as immediates.
Thu, Jan 3, 6:44 AM
asb removed a child revision for D52298: [RISCV][MC] Add support for evaluating constant symbols as immediates: D52299: [RISCV][MC] Accept %lo and %pcrel_lo on operands to li.
Thu, Jan 3, 6:44 AM
asb updated the diff for D52299: [RISCV][MC] Accept %lo and %pcrel_lo on operands to li.

Rebasing patch so it is no longer dependent on D52298

Thu, Jan 3, 6:44 AM
asb updated the diff for D52298: [RISCV][MC] Add support for evaluating constant symbols as immediates.

As suggested my Shiva, this patch has been updated to allow AsmParser::parseExpression to do the work when parsing an identifier.

Thu, Jan 3, 6:25 AM
asb added a child revision for D56264: [RISCV] Add patterns for RV64I SLLW/SRLW/SRAW instructions: D53230: [RISCV] Introduce codegen patterns for RV64M-only instructions.
Thu, Jan 3, 3:50 AM
asb added a parent revision for D53230: [RISCV] Introduce codegen patterns for RV64M-only instructions: D56264: [RISCV] Add patterns for RV64I SLLW/SRLW/SRAW instructions.
Thu, Jan 3, 3:50 AM
asb updated the diff for D53230: [RISCV] Introduce codegen patterns for RV64M-only instructions.

Updated the patch to address the correctness issues with the udiv and urem patterns. Like with D56264, a dag combine converts an ANY_EXTEND to a SIGN_EXTEND when it operates on an instruction where a 32-bit *W variant could be selected. This is advantageous as it can avoid unnecessary masking/sign-extension of the input operands.

Thu, Jan 3, 3:50 AM
asb created D56264: [RISCV] Add patterns for RV64I SLLW/SRLW/SRAW instructions.
Thu, Jan 3, 3:46 AM

Thu, Dec 20

asb added a comment to D55325: [RISCV] Add assembler support for LA pseudo-instruction.

Thanks, this basically looks good to me. Could you please add some tests for la with invalid operands.

Thu, Dec 20, 7:08 AM
asb accepted D55279: [RISCV] Support assembling %got_pcrel_hi operator.

This no longer seems to apply cleanly (needs rebase), but otherwise LGTM. Thanks!

Thu, Dec 20, 6:57 AM
asb committed rL349764: [RISCV] Properly evaluate fixup_riscv_pcrel_lo12.
[RISCV] Properly evaluate fixup_riscv_pcrel_lo12
Thu, Dec 20, 6:56 AM
asb closed D54029: [RISCV] Properly evaluate fixup_riscv_pcrel_lo12.
Thu, Dec 20, 6:55 AM

Dec 13 2018

asb added a comment to D54029: [RISCV] Properly evaluate fixup_riscv_pcrel_lo12.

I personally would regard that as an implementation detail. There's no reason we couldn't give an error; we bail out of shouldForceRelocation early without bothering to check the relocation if relaxation is enabled, but there's nothing stopping us from putting the %pcrel_lo error check before that other than being slightly slower and increasing code complexity.

Dec 13 2018, 8:36 AM
asb planned changes to D53230: [RISCV] Introduce codegen patterns for RV64M-only instructions.

Marking as "planned changes". At least the following patterns are problematic:

Dec 13 2018, 4:02 AM
asb accepted D54029: [RISCV] Properly evaluate fixup_riscv_pcrel_lo12.

This looks good to me, but there's one addition test I'd like to see. You explain that the intended behaviour is that there will be no compile-time error if linker relaxation is enabled. It would be good to have this reflected in a test. Maybe adding extra RUN lines to pcrel-lo12-invalid.s and adding a comment that explains that is expected with linker relaxation enabled.

Dec 13 2018, 3:39 AM
asb added a comment to D54205: [RISCV] Add support for the various RISC-V FMA instruction variants.

Forgot to change the status in Phabricator prior to commit, but I did re-review the updated change and it definitely LGTM!

Dec 13 2018, 2:56 AM
asb committed rL349023: [RISCV] Add support for the various RISC-V FMA instruction variants.
[RISCV] Add support for the various RISC-V FMA instruction variants
Dec 13 2018, 2:53 AM
asb closed D54205: [RISCV] Add support for the various RISC-V FMA instruction variants.
Dec 13 2018, 2:53 AM

Dec 12 2018

asb committed rL348929: [clang-fuzzer] Add explicit dependency on clangSerialization for clangHandleCXX….
[clang-fuzzer] Add explicit dependency on clangSerialization for clangHandleCXX…
Dec 12 2018, 6:36 AM
asb committed rC348929: [clang-fuzzer] Add explicit dependency on clangSerialization for clangHandleCXX….
[clang-fuzzer] Add explicit dependency on clangSerialization for clangHandleCXX…
Dec 12 2018, 6:36 AM

Dec 6 2018

asb added inline comments to D52416: Allow FP types for atomicrmw xchg.
Dec 6 2018, 9:45 AM
asb added inline comments to D54029: [RISCV] Properly evaluate fixup_riscv_pcrel_lo12.
Dec 6 2018, 6:34 AM

Dec 5 2018

asb updated subscribers of D55305: [RISCV] Add lowering of global TLS addresses.

Thanks for sharing this patch series Lewis. As it happens @luismarques has been finishing off his own implementation of TLS support which this unfortunately clashes with. The upside is that we're in a good position to review this work!

Dec 5 2018, 2:13 AM

Dec 4 2018

asb added a comment to D55279: [RISCV] Support assembling %got_pcrel_hi operator.

@rogfer01: James did actually get a patch for this committed to riscv-asm-manual https://github.com/riscv/riscv-asm-manual/blob/master/riscv-asm.md

Dec 4 2018, 10:59 AM

Dec 3 2018

asb added a comment to rL347988: [RISCV] Add UNIMP instruction (32- and 16-bit forms).

Thanks, fixed in rL348117.

Dec 3 2018, 2:39 AM
asb committed rL348117: [RISCV] Fix test/MC/Disassembler/RISCV/invalid-instruction.txt after rL347988.
[RISCV] Fix test/MC/Disassembler/RISCV/invalid-instruction.txt after rL347988
Dec 3 2018, 2:38 AM

Nov 30 2018

asb added inline comments to D52977: [RISCV] Introduce codegen patterns for instructions introduced in RV64I.
Nov 30 2018, 9:22 PM
asb added inline comments to D52977: [RISCV] Introduce codegen patterns for instructions introduced in RV64I.
Nov 30 2018, 9:19 PM
asb committed rL348067: [RISCV] Remove RV64I SLLW/SRLW/SRAW patterns and add new test cases.
[RISCV] Remove RV64I SLLW/SRLW/SRAW patterns and add new test cases
Nov 30 2018, 9:03 PM
asb planned changes to D53233: [RISCV] Add codegen support for RV64A.

I'll update this to include cmpxchg support now D48131 landed.

Nov 30 2018, 7:14 PM
asb requested changes to D54205: [RISCV] Add support for the various RISC-V FMA instruction variants.

Just some minor tweaks and I think this is good to go. In addition to the changes mentioned in inline comments, please:

  • add nounwind to all the test functions
  • Remove tail from the calls to llvm.fma as it doesn't impact the generated code (I have a preference to remove everything that doesn't impact the generated code).
Nov 30 2018, 7:04 AM
asb committed rL347991: [RISCV] Add additional CSR instruction aliases (imm. operands).
[RISCV] Add additional CSR instruction aliases (imm. operands)
Nov 30 2018, 6:13 AM
asb closed D55008: [RISCV] Add CSR instruction aliases (imm. operands).
Nov 30 2018, 6:13 AM
asb accepted D55008: [RISCV] Add CSR instruction aliases (imm. operands).

Looks good to me. I double-checked that gas doesn't do this same conversion for fsflags and fsrm. It doesn't, so this seems complete.

Nov 30 2018, 5:55 AM
asb committed rL347988: [RISCV] Add UNIMP instruction (32- and 16-bit forms).
[RISCV] Add UNIMP instruction (32- and 16-bit forms)
Nov 30 2018, 5:42 AM
asb closed D54316: [RISCV] Add UNIMP instruction (32- and 16-bit forms).
Nov 30 2018, 5:42 AM
asb accepted D54316: [RISCV] Add UNIMP instruction (32- and 16-bit forms).

Thanks to Luís, there is agreement on c.unimp on the sw-dev mailing list, it's now documented in the RISC-V asm manual, and binutils now supports c.unimp too (courtesy of Jim Wilson).

Nov 30 2018, 5:38 AM
asb committed rL347986: [SelectionDAG] Support result type promotion for FLT_ROUNDS_.
[SelectionDAG] Support result type promotion for FLT_ROUNDS_
Nov 30 2018, 5:21 AM
asb closed D53820: [SelectionDAG] Support result type promotion for FLT_ROUNDS_.
Nov 30 2018, 5:21 AM
asb accepted D53820: [SelectionDAG] Support result type promotion for FLT_ROUNDS_.

LGTM. Needs a trivial tweak to apply. This can land now the RV64I patch was committed. I'll commit now (I believe Lewis doesn't have commit access).

Nov 30 2018, 5:14 AM
asb updated the diff for D54574: [SelectionDAG] Support promotion of the FPOWI integer operand.

Updated to add note about FPOWI being undefined if integer operand is > 32-bits, as suggested by @efriedma

Nov 30 2018, 3:59 AM
asb added a comment to D53235: [RISCV] Add RV64F codegen support.

Submitting some old in-line comments that were accidentally left unsubmitted.

Nov 30 2018, 3:52 AM
asb committed rL347980: [SelectionDAG] Support promotion of PREFETCH operands.
[SelectionDAG] Support promotion of PREFETCH operands
Nov 30 2018, 2:09 AM
asb closed D53281: [SelectionDAG] Support promotion of PREFETCH operands.
Nov 30 2018, 2:09 AM
asb committed rL347978: [SelectionDAG] Support promotion of FRAMEADDR/RETURNADDR operands.
[SelectionDAG] Support promotion of FRAMEADDR/RETURNADDR operands
Nov 30 2018, 2:06 AM
asb closed D53279: [SelectionDAG] Support promotion of FRAMEADDR/RETURNADDR operands.
Nov 30 2018, 2:06 AM
asb committed rL347977: [TargetLowering][RISCV] Introduce isSExtCheaperThanZExt hook and implement for….
[TargetLowering][RISCV] Introduce isSExtCheaperThanZExt hook and implement for…
Nov 30 2018, 2:01 AM
asb closed D52978: [TargetLowering][RISCV] Introduce isSExtCheaperThanZExt hook and implement for RISC-V.
Nov 30 2018, 2:00 AM
asb committed rL347973: [RISCV] Introduce codegen patterns for instructions introduced in RV64I.
[RISCV] Introduce codegen patterns for instructions introduced in RV64I
Nov 30 2018, 1:42 AM
asb closed D52977: [RISCV] Introduce codegen patterns for instructions introduced in RV64I.
Nov 30 2018, 1:42 AM
asb committed rL347971: [docs][AtomicExpandPass] Document the alternate lowering strategy for part-word….
[docs][AtomicExpandPass] Document the alternate lowering strategy for part-word…
Nov 30 2018, 1:26 AM
asb closed D52234: [docs][AtomicExpandPass] Document the alternate lowering strategy for part-word atomicrmw/cmpxchg.
Nov 30 2018, 1:26 AM

Nov 29 2018

asb committed rL347914: [RISCV] Implement codegen for cmpxchg on RV32IA.
[RISCV] Implement codegen for cmpxchg on RV32IA
Nov 29 2018, 12:46 PM
asb closed D48131: [RISCV] Implement codegen for cmpxchg on RV32IA.
Nov 29 2018, 12:46 PM
asb added reviewers for D52416: Allow FP types for atomicrmw xchg: asb, jyknight.

I should probably plan to reflect support for this in my in-flight cmpxchg patch (which adds target-independent support for late lowering of cmpxchg in the same way I added it for atomicrmw). [incidentally - reviews on that patch would be very welcome...]

Nov 29 2018, 7:28 AM
asb added a comment to D48131: [RISCV] Implement codegen for cmpxchg on RV32IA.

This patch still applies with minimal fuzz and all tests still pass.

Nov 29 2018, 6:25 AM
asb added a comment to D50496: [RISCV] Implment pseudo instructions for load/store from a symbol address..
  • I'd like to find a solution that doesn't get rid of the helpful error reporting we currently have for load/store. Downgrading all errors to just "invalid operand for instruction" is an unfortunate regression
  • The F/D load/store are wrong as this patch has them accepting GPRs rather than FPR32/FPR64. The outs for PseudoSTORE also has a mistake
  • We no longer have the shouldForceImmediate logic because associating custom parsers with operands was found to be superior. From some playing with a modified version of this patch it looks like more logic is needed to get these aliases to parse though...
Nov 29 2018, 3:21 AM
asb accepted D50141: Add errors for tiny codemodel on targets other than AArch64.

Looks good to me.

Nov 29 2018, 2:02 AM

Nov 28 2018

asb committed rL347774: [RISCV] Support .option push and .option pop.
[RISCV] Support .option push and .option pop
Nov 28 2018, 8:42 AM
asb closed D46424: [RISCV] Support .option push and .option pop.
Nov 28 2018, 8:42 AM
asb accepted D46424: [RISCV] Support .option push and .option pop.
Nov 28 2018, 8:41 AM
asb added a comment to D54316: [RISCV] Add UNIMP instruction (32- and 16-bit forms).

I think this is fine, but c.unimp should be proposed on the RISC-V sw-dev mailing list or similar. As we discussed, it seems to make total sense to have this instruction (every other compressed instruction is nameable with c.something). But we should get community feedback first before merging in to LLVM. It's probably also worth highlighting the fact that "unimp" isn't actually guaranteed to trap according to the current RISC-V specs...

Nov 28 2018, 8:27 AM