t.p.northover (Tim Northover)
Lord High Supreme Bullshitter

Projects

User does not belong to any projects.

User Details

User Since
Oct 18 2012, 4:53 AM (295 w, 4 d)

Recent Activity

Today

t.p.northover updated the diff for D45321: [atomics] Fix runtime calls for misaligned atomics.

Fixing cmpxchg implementation of load to return correct value.

Mon, Jun 18, 5:09 AM
t.p.northover added a comment to D45321: [atomics] Fix runtime calls for misaligned atomics.

Sorry I take so long replying to this each time. I really need to plan my time better.

Mon, Jun 18, 5:06 AM
t.p.northover updated the diff for D48170: ARM: use "add" instead of "orr" for code size.

Switched to T1Pat on Thumb-1 side and added t2ADDrr pattern.

Mon, Jun 18, 3:17 AM
t.p.northover added inline comments to D48170: ARM: use "add" instead of "orr" for code size.
Mon, Jun 18, 3:16 AM

Fri, Jun 15

t.p.northover added inline comments to D48170: ARM: use "add" instead of "orr" for code size.
Fri, Jun 15, 2:32 AM

Thu, Jun 14

t.p.northover created D48170: ARM: use "add" instead of "orr" for code size.
Thu, Jun 14, 6:34 AM
t.p.northover added a comment to D48130: [AtomicExpandPass]: Add a hook for custom cmpxchg expansion in IR.

It seems a bit of a shame to hide that masked expansion away in lib/Target/RISCV when MIPS (at least) needs similar logic. I suppose someone who wants to refactor MIPS can move it out again though.

Thu, Jun 14, 6:22 AM
t.p.northover added inline comments to D48131: [RISCV] Implement codegen for cmpxchg on RV32I.
Thu, Jun 14, 6:22 AM
t.p.northover added a comment to D48131: [RISCV] Implement codegen for cmpxchg on RV32I.

I obviously haven't been following RISC-V enough to give final reviews, but this mostly looked sensible to me. Just one question:

Thu, Jun 14, 6:08 AM
t.p.northover accepted D48056: [AArch64] Implement FLT_ROUNDS macro.

I think this looks reasonable to me.

Thu, Jun 14, 5:31 AM

Thu, May 24

t.p.northover accepted D46844: [AArch64] Take advantage of variable shift/rotate amount implicit mod operation..

I think this is fine.

Thu, May 24, 5:36 AM
t.p.northover accepted D47176: [AArch64] Improve orr+movk sequences for MOVi64imm..

Looks reasonable to me.

Thu, May 24, 5:17 AM

May 11 2018

Herald added a reviewer for D45321: [atomics] Fix runtime calls for misaligned atomics: javed.absar.

The Linux libatomic __atomic_is_lock_free returns false for unaligned pointers, even on x86. clang must generate code which is compatible with that, so it *cannot* inline misaligned atomic operations.

May 11 2018, 6:08 AM

Apr 26 2018

t.p.northover added a comment to D45770: [AArch64] Disable spill slot scavenging when stack realignment required..

I think I prefer Geoff's solution too. Better to make it work than just turn off parts of the optimizer, and it's not even particularly more difficult to read.

Apr 26 2018, 6:44 AM

Apr 23 2018

t.p.northover closed D45319: [Atomics] warn about misaligned atomic accesses using libcalls.

Ah, I see you approved it before, presumably with the suggested changes. I've committed as r330566.

Apr 23 2018, 1:20 AM · Restricted Project
t.p.northover updated the diff for D45319: [Atomics] warn about misaligned atomic accesses using libcalls.

Moved diagnostic emission next to where the decision is made.

Apr 23 2018, 1:18 AM · Restricted Project
t.p.northover accepted D45199: AArch64: Allow offsets to be folded into addresses with ELF..

Yep, I think that ought to work. Sorry about the delay.

Apr 23 2018, 12:57 AM

Apr 19 2018

t.p.northover added inline comments to D45319: [Atomics] warn about misaligned atomic accesses using libcalls.
Apr 19 2018, 6:26 AM · Restricted Project

Apr 13 2018

t.p.northover updated the diff for D45321: [atomics] Fix runtime calls for misaligned atomics.
Apr 13 2018, 2:46 AM
t.p.northover updated the diff for D45321: [atomics] Fix runtime calls for misaligned atomics.

Sorry to update the patch after you've reviewed it, but I'm afraid as a result of https://llvm.org/PR34347 it's turned out that misaligned x86 atomics actually need the lock-free implementation, which makes this even messier than I thought.

Apr 13 2018, 2:46 AM
t.p.northover added a comment to D45319: [Atomics] warn about misaligned atomic accesses using libcalls.

Ping.

Apr 13 2018, 12:36 AM · Restricted Project

Apr 12 2018

t.p.northover accepted D45199: AArch64: Allow offsets to be folded into addresses with ELF..

Thanks for updating the patch. LGTM!

Apr 12 2018, 1:35 PM
t.p.northover added inline comments to D45199: AArch64: Allow offsets to be folded into addresses with ELF..
Apr 12 2018, 12:00 PM

Apr 10 2018

t.p.northover accepted D45483: [NEON] Support vfma_n and vfms_n intrinsics.

Looks fine to me.

Apr 10 2018, 6:59 AM · Restricted Project
t.p.northover accepted D45462: Add missing nullptr check before getSection() to AArch64MachObjectWriter::recordRelocation.

LGTM.

Apr 10 2018, 6:38 AM

Apr 9 2018

t.p.northover updated the diff for D45321: [atomics] Fix runtime calls for misaligned atomics.

Updated so that 16-byte atomics work when the instruction is always available. Good idea Eli.

Apr 9 2018, 1:43 AM
t.p.northover accepted D45199: AArch64: Allow offsets to be folded into addresses with ELF..

Fair enough, thanks for looking into it. LGTM!

Apr 9 2018, 12:55 AM

Apr 7 2018

t.p.northover closed D42898: Do not spill CSR to stack on entry to noreturn functions.
Apr 7 2018, 4:01 AM
t.p.northover added a comment to D42898: Do not spill CSR to stack on entry to noreturn functions.

I managed to track down the issue, and I think the patch needs to respect the "uwtable" attribute too (otherwise the tables produced are useless). So I've recommitted with that change applied; hopefully it'll stick this time.

Apr 7 2018, 4:01 AM

Apr 6 2018

t.p.northover added inline comments to D45321: [atomics] Fix runtime calls for misaligned atomics.
Apr 6 2018, 11:15 AM
t.p.northover added inline comments to D45321: [atomics] Fix runtime calls for misaligned atomics.
Apr 6 2018, 9:25 AM

Apr 5 2018

t.p.northover added a comment to D45321: [atomics] Fix runtime calls for misaligned atomics.

Bug won't clang lower these access to the exactly that IR that LLVM declares as UB?

Apr 5 2018, 9:37 AM
t.p.northover added a comment to D45321: [atomics] Fix runtime calls for misaligned atomics.

Also, on the LLVM side there are even fewer restrictions and load atomic i32, i32* %ptr monotonic, align 1 is perfectly valid IR that gets lowered to these calls.

Apr 5 2018, 7:36 AM
t.p.northover closed D42898: Do not spill CSR to stack on entry to noreturn functions.

Sorry about the delay, I've committed this for you as r329287.

Apr 5 2018, 7:30 AM
t.p.northover added a comment to D45321: [atomics] Fix runtime calls for misaligned atomics.

Atomics accessed via C11 _Atomic and C++11 std::atomic will be suitably aligned, but there's a reasonable amount of legacy code that uses the GCC builtins on non-atomic types (of unknown alignment) and this is what Clang uses to implement those accesses when they come up. Also, on the LLVM side there are even fewer restrictions and load atomic i32, i32* %ptr monotonic, align 1 is perfectly valid IR that gets lowered to these calls.

Apr 5 2018, 7:26 AM
t.p.northover added inline comments to D45199: AArch64: Allow offsets to be folded into addresses with ELF..
Apr 5 2018, 6:53 AM
t.p.northover created D45321: [atomics] Fix runtime calls for misaligned atomics.
Apr 5 2018, 6:36 AM
t.p.northover created D45319: [Atomics] warn about misaligned atomic accesses using libcalls.
Apr 5 2018, 6:21 AM · Restricted Project

Mar 27 2018

t.p.northover added a comment to D44815: [AArch64]: Add support for parsing rN registers..

The warning you're seeing is because without an operand modifier Clang always chooses an x-register in its textual assembly expansion. This 64-bit default is compared to the underlying C type to determine whether a warning should be emitted, and I think that's reasonable. The C type is all that's available for most uses (without a register keyword) and it seems consistent to use it even when register has been specified on the variable.

Mar 27 2018, 6:16 AM

Mar 24 2018

t.p.northover accepted D44851: [AArch64] Decorate AArch64 instrs with OPERAND_PCREL.

A nice little change with big benefits! One little nit (no need to reupload here after fixing, I think):

Mar 24 2018, 4:06 AM

Mar 23 2018

t.p.northover accepted D44709: [ARM] Fix "Constant pool entry out of range!" in Thumb1 mode.

Looks like it's clearly an improvement, and very low risk so it'd just as well go into 6.0 if there's time.

Mar 23 2018, 6:30 AM
t.p.northover accepted D44794: [AArch64] Don't reduce the width of loads if it prevents combining a shift.

Looks reasonable to me.

Mar 23 2018, 6:28 AM

Mar 19 2018

t.p.northover accepted D44538: [ARM] Support for v4f16 and v8f16 vectors.

Thanks. This looks fine now too.

Mar 19 2018, 4:58 AM
t.p.northover accepted D44561: [ARM] Pass half or i16 types for NEON intrinsics.

Thanks. I think this looks reasonable now.

Mar 19 2018, 3:52 AM

Mar 17 2018

t.p.northover accepted D44586: [AArch64] Skip an unnecessary getCopyToReg in DYNAMIC_STACKALLOC.

Looks fine to me.

Mar 17 2018, 5:22 AM

Mar 16 2018

t.p.northover added inline comments to D44573: [AArch64] Add patterns matching (fabs (fsub x y)) to (fabd x y).
Mar 16 2018, 11:13 AM
t.p.northover added inline comments to D44561: [ARM] Pass half or i16 types for NEON intrinsics.
Mar 16 2018, 9:10 AM
t.p.northover added inline comments to D44561: [ARM] Pass half or i16 types for NEON intrinsics.
Mar 16 2018, 7:41 AM
t.p.northover added a comment to D44538: [ARM] Support for v4f16 and v8f16 vectors.

Yep, I thought the bitconverts were a good idea, but should also have the variants that trigger for IsBE.

Mar 16 2018, 7:19 AM
t.p.northover added inline comments to D44561: [ARM] Pass half or i16 types for NEON intrinsics.
Mar 16 2018, 7:18 AM
t.p.northover added a comment to D44544: llvm-objdump: Print symbol name if it's address is the same as the next one's.

Why is this better?

Mar 16 2018, 6:00 AM
t.p.northover added a comment to D44538: [ARM] Support for v4f16 and v8f16 vectors.

Ah good, thanks for the explanation. Sounds like you were already doing exactly what I was thinking of and I started paying attention half-way through.

Mar 16 2018, 5:55 AM

Mar 15 2018

t.p.northover added a comment to D44538: [ARM] Support for v4f16 and v8f16 vectors.

It looks like there's nothing testing the bitconvert patterns here.

Mar 15 2018, 1:46 PM
t.p.northover accepted D44510: [AArch64] Codegen tests for the Armv8.2-A FP16 intrinsics.

These look fine to me.

Mar 15 2018, 5:26 AM
t.p.northover accepted D44512: [AAch64] Tests for ACLE FP16 macros.

These look right to me.

Mar 15 2018, 5:12 AM

Dec 10 2017

t.p.northover closed D40948: Switch Clang's default C++ language target to C++14.

Thanks Richard, and all other reviewers. I committed this as r320250, with a couple of sanitizer test fixes as r320251 and r320284 (thanks Ahmed!).

Dec 10 2017, 11:41 PM

Dec 8 2017

t.p.northover added inline comments to D40948: Switch Clang's default C++ language target to C++14.
Dec 8 2017, 7:43 AM
t.p.northover added inline comments to D40948: Switch Clang's default C++ language target to C++14.
Dec 8 2017, 6:06 AM
t.p.northover updated the diff for D40948: Switch Clang's default C++ language target to C++14.

Updating with tentative fixes to review comments.

Dec 8 2017, 5:31 AM

Dec 7 2017

t.p.northover added a comment to D40948: Switch Clang's default C++ language target to C++14.

Thanks Richard. I'll file the bugs tomorrow for the issues you suggest. Do you see either of them blocking the change to C++14 as a default? On a scale of "no", "no but I want a commitment to fix them" and "yes" sort of thing.

Dec 7 2017, 2:19 PM
t.p.northover added inline comments to D40948: Switch Clang's default C++ language target to C++14.
Dec 7 2017, 12:49 PM
t.p.northover created D40948: Switch Clang's default C++ language target to C++14.
Dec 7 2017, 3:26 AM

Oct 25 2017

t.p.northover accepted D30529: [RFC][GlobalISel] Enable legalizing non-power-of-2 sized types..

Thanks Kristof. I think this looks pretty reasonable as a starting point.

Oct 25 2017, 6:27 AM

Sep 22 2017

t.p.northover accepted D36306: [ARM] Fix assembly and disassembly for VMRS/VMSR.
Thanks, this new one looks good too.
Sep 22 2017, 1:16 AM

Sep 18 2017

t.p.northover added a comment to D37968: [ARM] Fix for indexed dot product instruction descriptions.

Where is the restriction that only the lower 16 registers are allowed? Can you test quad regs, too? Just to make it clear the lane issue.

Sep 18 2017, 7:01 AM

Sep 5 2017

t.p.northover accepted D37493: Fix ARM bare metal driver to support atomics.

Oh hang on, I see you're undoing something which broke my assumptions. In which case, I completely agree with everything.

Sep 5 2017, 2:49 PM
t.p.northover added a comment to D37493: Fix ARM bare metal driver to support atomics.

I think this is wrong. "arm-none-eabi" covers RTOS threads too, where you really should use ldrex/strex loops unless you have good reason; and even kernels can be interrupted if they decide they want it.

Sep 5 2017, 2:48 PM

Aug 28 2017

t.p.northover accepted D36507: [ARM] GlobalISel: Select globals in PIC mode.

Looks reasonable to me.

Aug 28 2017, 7:45 AM
t.p.northover accepted D37030: Fix ARMv4 support.

I think the issues have been basically resolved here. The assertion can't hurt. LGTM!

Aug 28 2017, 7:35 AM

Aug 22 2017

t.p.northover accepted D37040: LowerAtomic: Don't skip optnone functions; atomic still need lowering (PR34020).

LGTM.

Aug 22 2017, 5:26 PM
t.p.northover added a comment to D37020: [AArch64][Falkor] Avoid generating STRQro* instructions.

There should be a test, but other than that only one nit (also with the comment).

Aug 22 2017, 1:47 PM

Aug 21 2017

t.p.northover added a comment to D36830: Use report_fatal_error for unsupported calling conventions.

I think it's a reasonable change. llvm_unreachable is clearly problematic.

Aug 21 2017, 1:55 PM

Aug 14 2017

t.p.northover accepted D36667: [ARM][AArch64] Cortex-A75 and Cortex-A55 support.

Looks reasonable to me. Looking forward to the RCPC instructions!

Aug 14 2017, 7:45 AM

Aug 10 2017

t.p.northover added a comment to D36575: [ARM] Assembler support for the ARMv8.2a dot product instructions.

Shouldn't the feature be predicated on HasV8_2aOps, not just NEON? It seems weird, from the tests, that we can target arm or thumbv7 but still have access to these instructions.

Aug 10 2017, 8:58 AM
t.p.northover added a comment to D36576: Fix -fPIC handling on arm64.

Also, make sure "llvm-commits" is a subscriber.

Aug 10 2017, 6:57 AM
t.p.northover updated subscribers of D36576: Fix -fPIC handling on arm64.
Aug 10 2017, 6:56 AM

Aug 9 2017

t.p.northover added a comment to D36522: [AArch64] Assembler support for v8.3 RCpc.

Looks good. Thanks.

Aug 9 2017, 7:59 AM
t.p.northover accepted D36515: [AArch64] Assembler support for the ARMv8.2a dot product instructions.

Looks reasonable to me.

Aug 9 2017, 7:45 AM
t.p.northover accepted D36522: [AArch64] Assembler support for v8.3 RCpc.

LGTM with one nit.

Aug 9 2017, 7:29 AM

Aug 8 2017

t.p.northover added a comment to D35817: Ban implicit _Complex to scalar conversions in C++.

What's going on with the OpenMP test changes?

Aug 8 2017, 12:13 PM
t.p.northover added a comment to D35817: Ban implicit _Complex to scalar conversions in C++.

Pingy.

Aug 8 2017, 11:34 AM

Aug 4 2017

t.p.northover accepted D36306: [ARM] Fix assembly and disassembly for VMRS/VMSR.

Looks reasonable to me, though please remember to add llvm-commits as a subscriber next time.

Aug 4 2017, 1:48 PM
t.p.northover added a comment to D36272: [CodeGen][x86_64] Enable 'force_align_arg_pointer' attribute at x86_64.

I've added "cfe-commits" as a subscriber since that's how we make sure the review request gets to the mailing list (which is still where review canonically takes place).

Aug 4 2017, 1:30 PM · Restricted Project
t.p.northover updated subscribers of D36272: [CodeGen][x86_64] Enable 'force_align_arg_pointer' attribute at x86_64.
Aug 4 2017, 1:29 PM · Restricted Project
t.p.northover accepted D35319: LSE Atomics reorg - Part I.

Thanks for going through so many revisions on this. I think it looks good now.

Aug 4 2017, 7:40 AM

Aug 2 2017

t.p.northover accepted D35883: [ARM] GlobalISel: Select simple G_GLOBAL_VALUE instructions.

Looks reasonable to me.

Aug 2 2017, 10:41 AM
t.p.northover accepted D36223: [AArch64] Simplify AES*Tied pseudo expansion (NFC)..

LGTM.

Aug 2 2017, 7:58 AM

Aug 1 2017

t.p.northover accepted D35919: [AArch64] Rewrite stack frame handling for win64 vararg functions.

Thanks Martin, I think this looks reasonable now too.

Aug 1 2017, 1:56 PM
t.p.northover accepted D36174: [AArch64] Fix a typo in isExtFreeImpl().

LGTM

Aug 1 2017, 1:39 PM
t.p.northover added inline comments to D35919: [AArch64] Rewrite stack frame handling for win64 vararg functions.
Aug 1 2017, 1:00 PM
t.p.northover updated the diff for D35817: Ban implicit _Complex to scalar conversions in C++.

Sounds like an improvement, I've updated the diff.

Aug 1 2017, 10:11 AM

Jul 31 2017

t.p.northover added a comment to D35970: Teach cc1as driver to accept ARM ropi/rwpi reloc model.

This looks like it'll get the build attributes wrong (since only PIC gets plumbed through to the backend). That's probably worse than simply rejecting the arguments.

Jul 31 2017, 3:56 PM
t.p.northover accepted D33435: [SelectionDAG] reset NewNodesMustHaveLegalTypes flag between basic blocks .

I'm really sorry this took so long. This patch is obviously an improvement. A PR against AArch64 would be sort of nice, but I don't think any more is needed because the actual chances of it affecting real codegen seems pretty small (significant vector code in the entry block?).

Jul 31 2017, 10:34 AM
t.p.northover added a comment to D35817: Ban implicit _Complex to scalar conversions in C++.

Ping.

Jul 31 2017, 10:00 AM
t.p.northover accepted D35737: [GSel]: Support Widening G_ICMP's destination operand..

Thanks. Looks reasonable to me.

Jul 31 2017, 9:27 AM

Jul 28 2017

t.p.northover added a comment to D35737: [GSel]: Support Widening G_ICMP's destination operand..

AArch64 has no 16-bit instructions (at least outside the SIMD unit). The first sensible type to legalize to is s32.

Jul 28 2017, 7:12 PM
t.p.northover accepted D35299: [AArch64] Tie source and destination operands for AESMC/AESIMC. .

Yep, I'd be happy with that too. Commit away!

Jul 28 2017, 1:59 PM
t.p.northover added a comment to D35299: [AArch64] Tie source and destination operands for AESMC/AESIMC. .

Not convinced by a Hungarian tagging like that. It's not what happens with existing pseudo-instructions. If anything they have a semi-descriptive name: "MOVaddr", "MOVaddrJT", "CMP_SWAP_8", ...

Jul 28 2017, 9:32 AM
t.p.northover added inline comments to D34873: Fix miscompiled 32bit binaries by mingw.
Jul 28 2017, 9:29 AM

Jul 27 2017

t.p.northover accepted D35927: [NFC] standardized suffixes for LSE Atomics mnemonics.

Looks fine. Thanks for splitting it out.

Jul 27 2017, 6:50 AM