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 (309 w, 2 d)

Recent Activity

Fri, Sep 21

t.p.northover accepted D52335: AArch64FastISel: Abort intrinsic selection if the left operand didn't select.

I've had a go at producing an example too with no luck. The Constant involved on the breaking config is ridiculously complex (even after reduction): i32 or (i32 shl (i32 or (i32 shl (i32 or (i32 shl (i32 and (i32 ptrtoint ([19 x i8]* @0 to i32), i32 63), i32 8), i32 and (i32 lshr (i32 ptrtoint ([19 x i8]* @0 to i32), i32 6), i32 63)), i32 8), i32 and (i32 lshr (i32 lshr (i32 ptrtoint ([19 x i8]* @0 to i32), i32 6), i32 6), i32 63)), i32 8), i32 lshr (i32 lshr (i32 lshr (i32 ptrtoint ([19 x i8]* @0 to i32), i32 6), i32 6), i32 6))

Fri, Sep 21, 6:14 AM
t.p.northover updated the diff for D32564: AArch64: compress jump tables to minimum size needed to reach destinations.

Thread necromancy! I realised that I got distracted and never finished this discussion.

Fri, Sep 21, 4:29 AM

Thu, Sep 20

t.p.northover added inline comments to D48131: [RISCV] Implement codegen for cmpxchg on RV32I.
Thu, Sep 20, 4:52 AM

Thu, Sep 13

t.p.northover accepted D51780: ARM: align loops to 4 bytes on Cortex-M3 and Cortex-M4..

Thanks. Committed as r342127.

Thu, Sep 13, 3:56 AM

Wed, Sep 12

t.p.northover updated the diff for D51780: ARM: align loops to 4 bytes on Cortex-M3 and Cortex-M4..

I think I've implemented the suggested changes, except for the function alignment one.

Wed, Sep 12, 4:13 AM

Mon, Sep 10

t.p.northover added a comment to D51780: ARM: align loops to 4 bytes on Cortex-M3 and Cortex-M4..

We should also be aligning functions?

Mon, Sep 10, 5:09 AM

Fri, Sep 7

t.p.northover created D51780: ARM: align loops to 4 bytes on Cortex-M3 and Cortex-M4..
Fri, Sep 7, 5:27 AM
t.p.northover closed D51678: ARM: fix Thumb2 CodeGen for ldrex with folded frame-index.

Thanks. Committed as r341642.

Fri, Sep 7, 5:21 AM

Thu, Sep 6

t.p.northover added inline comments to D51678: ARM: fix Thumb2 CodeGen for ldrex with folded frame-index.
Thu, Sep 6, 2:40 PM
t.p.northover updated the diff for D51678: ARM: fix Thumb2 CodeGen for ldrex with folded frame-index.

Added the addressing-mode to ARMFrameLowering. The only other place that uses them appears to be optional (for an efficiency gain), so I skipped that.

Thu, Sep 6, 5:57 AM

Wed, Sep 5

t.p.northover added a comment to D51678: ARM: fix Thumb2 CodeGen for ldrex with folded frame-index.

Bother, yes I do. That's going to be fun to write a test for so it'll probably have to wait until tomorrow.

Wed, Sep 5, 11:09 AM
t.p.northover accepted D51630: [DAGCombiner] try to convert pow(x, 0.25) to sqrt(sqrt(x)).

Thanks! LGTM.

Wed, Sep 5, 8:35 AM
t.p.northover added a comment to D51630: [DAGCombiner] try to convert pow(x, 0.25) to sqrt(sqrt(x)).

They'd probably work. We often just use useful triples, maybe thumbv8-linux-gnueabihf, and perhaps thumbv7m-linux-gnueabi as a soft-float target that ought to use libcalls for everything (I'd probably just check there are the appropriate number of calls there rather than tracking all the marshalling nonsense that goes on).

Wed, Sep 5, 7:39 AM
t.p.northover accepted D51683: Fix arm_neon.h and arm_fp16.h generation for compiling with std=c89.

Looks good to me.

Wed, Sep 5, 7:38 AM
t.p.northover added a comment to D51630: [DAGCombiner] try to convert pow(x, 0.25) to sqrt(sqrt(x)).

Could you possibly duplicate the tests for ARM? It looks like the patch does the right thing to me, but it'd be good to have it confirmed and tested.

Wed, Sep 5, 5:48 AM
t.p.northover accepted D51631: [ARM] Tighten f64<->f16 conversion requirements.

LGTM!

Wed, Sep 5, 4:51 AM
t.p.northover created D51678: ARM: fix Thumb2 CodeGen for ldrex with folded frame-index.
Wed, Sep 5, 3:44 AM

Aug 23 2018

t.p.northover accepted D51202: [AArch64] Reject inline asm with FP registers when FP is disabled..

LGTM.

Aug 23 2018, 8:17 PM

Aug 20 2018

t.p.northover added a comment to D50979: Eliminate instances of `EmitScalarExpr(E->getArg(n))` in EmitX86BuiltinExpr()..

EmitAArch64BuiltinExpr() also emits args into Ops before the big switch (with some more subtlety around the last arg that I don't understand), but then almost every switch case does EmitScalarExpr(E->getArg(n)).

Aug 20 2018, 12:26 PM

Aug 6 2018

t.p.northover added a comment to D50232: Fix modules build with different technique to suppress Knuth debugging.

Thanks for the comments. I've committed basically the original version of the D40404 (modulo DEBUG -> LLVM_DEBUG renaming) as r339009.

Aug 6 2018, 4:43 AM

Aug 3 2018

t.p.northover created D50232: Fix modules build with different technique to suppress Knuth debugging.
Aug 3 2018, 2:21 AM

Aug 1 2018

t.p.northover accepted D43860: [AArch64] DWARF: do not generate AT_location for thread local.

I think it's reasonable too.

Aug 1 2018, 7:58 AM
t.p.northover added a comment to D49929: [AArch64] Disallow the MachO specific .loh directive for windows.

I suppose that this is caused by the pair-wise split relocations à la IMAGE_REL_ARM_MOV32Tfor PE/COFF

Aug 1 2018, 6:12 AM
t.p.northover accepted D50137: [AArch64] Add support for got relocated LDR's.

Looks fine to me.

Aug 1 2018, 6:02 AM

Jul 31 2018

t.p.northover added a comment to D49929: [AArch64] Disallow the MachO specific .loh directive for windows.

IMO, linker optimization hints are more an AArch64 thing than a Darwin thing. They were invented entirely to deal with AArch64's ADRP/whatever sequences so I'd prefer the implementation to stay in lib/Target/AArch64 where possible.

Jul 31 2018, 2:10 PM

Jul 18 2018

t.p.northover created D49485: CMake: use new policy for CMP0051.
Jul 18 2018, 7:27 AM
t.p.northover closed D49014: [Support] Build fix for Haiku when checking for a local filesystem.

I've committed this as r337389. Thanks for submitting the patch.

Jul 18 2018, 6:48 AM
t.p.northover added a comment to D48925: X86: add alias for pushfw/popfw in Intel mode.

Ping.

Jul 18 2018, 6:33 AM

Jul 6 2018

t.p.northover closed D48939: CallGraph passes: iterate over all functions rather than just externally visible ones.

Thanks Duncan. Committed as r336419.

Jul 6 2018, 1:12 AM

Jul 5 2018

t.p.northover updated the diff for D48939: CallGraph passes: iterate over all functions rather than just externally visible ones.

Address review comments and rework iteration to always find next location before modifying the current one.

Jul 5 2018, 7:34 AM
t.p.northover added a comment to D48939: CallGraph passes: iterate over all functions rather than just externally visible ones.

Thanks for looking at the patch.

Jul 5 2018, 7:33 AM

Jul 4 2018

t.p.northover created D48939: CallGraph passes: iterate over all functions rather than just externally visible ones.
Jul 4 2018, 6:52 AM
t.p.northover created D48925: X86: add alias for pushfw/popfw in Intel mode.
Jul 4 2018, 2:29 AM

Jun 28 2018

t.p.northover accepted D48674: SelectionDAGBuilder, mach-o: Skip trap after noreturn call (for Mach-O).

This looks reasonable to me.

Jun 28 2018, 1:39 AM

Jun 25 2018

t.p.northover added a comment to D46013: [ARM] Conform to AAPCS when passing overaligned composites as arguments.

I'd rather not put target names in API functions. The meaning of that field is pretty target independent ("alignment of type, before alignment adjustments")

Jun 25 2018, 2:30 AM

Jun 22 2018

t.p.northover added a comment to D46013: [ARM] Conform to AAPCS when passing overaligned composites as arguments.

I'm fine with the ABI changes, but I'm not very convinced by the "NaturalAlignment" name.

Jun 22 2018, 12:34 AM

Jun 20 2018

t.p.northover added a comment to D48056: [AArch64] Implement FLT_ROUNDS macro.

Sorry, I meant to do this a few days ago but forgot to actually commit and just rediscovered it today. It's pushed as r335118 now.

Jun 20 2018, 5:14 AM

Jun 19 2018

t.p.northover updated the diff for D48170: ARM: use "add" instead of "orr" for code size.

Added t2ADDi12 and tests for the immediate cases.

Jun 19 2018, 4:29 AM
t.p.northover added inline comments to D48170: ARM: use "add" instead of "orr" for code size.
Jun 19 2018, 4:28 AM

Jun 18 2018

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

Fixing cmpxchg implementation of load to return correct value.

Jun 18 2018, 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.

Jun 18 2018, 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.

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

Jun 15 2018

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

Jun 14 2018

t.p.northover created D48170: ARM: use "add" instead of "orr" for code size.
Jun 14 2018, 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.

Jun 14 2018, 6:22 AM
t.p.northover added inline comments to D48131: [RISCV] Implement codegen for cmpxchg on RV32I.
Jun 14 2018, 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:

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

I think this looks reasonable to me.

Jun 14 2018, 5:31 AM

May 24 2018

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

I think this is fine.

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

Looks reasonable to me.

May 24 2018, 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