Page MenuHomePhabricator

srj (Steven Johnson)
User

Projects

User does not belong to any projects.

User Details

User Since
Aug 23 2018, 5:53 PM (113 w, 3 d)

Recent Activity

Tue, Oct 13

srj added a comment to D34654: Allow passing a regex for headers to exclude from clang-tidy.

This is marked as "needs revision", but it's not clear to me what the requested changes actually are.

Tue, Oct 13, 6:29 PM · Restricted Project

Sep 15 2020

srj added a comment to rG0ee54cf88329: [Hexagon] Account for truncating pairs to non-pairs when widening truncates.

OK, here's a .ll that gives me different assembly results for llc @ 621b10ca187 and llc @ 0ee54cf8832 (and running it without Halide's harness produces numerically different results as well, which is really the entire problem). In both cases I just use llc -march=hexagon < halide_hvx64_example.ll.

Sep 15 2020, 4:13 PM
srj added a comment to rG0ee54cf88329: [Hexagon] Account for truncating pairs to non-pairs when widening truncates.

So far, I too am not seeing any differences when using llc on that bitcode, which is genuinely puzzling. To the best of my knowledge, we don't have any custom LLVM passes for Hexagon in Halide; we do have a number of custom passes that manipulate *Halide* IR, but at the end of the day, this IR all goes thru LLVM to generate Hexagon assembly, object code, etc.

Sep 15 2020, 2:42 PM
srj added a comment to rG0ee54cf88329: [Hexagon] Account for truncating pairs to non-pairs when widening truncates.

If you have the ability to pass LLVM options, could you get the good and bad outputs with -debug-only=isel,legalize-types,legalizedag -print-after-all?

Sep 15 2020, 11:30 AM
srj added a comment to rG0ee54cf88329: [Hexagon] Account for truncating pairs to non-pairs when widening truncates.

Just llc -march=hexagon < bad_hvx64_code.ll, but I added "target-cpu"="hexagonv65" "target-features"="+hvx,+hvx-length64b" to attributes inside the file.

Sep 15 2020, 11:02 AM
srj added a comment to rG0ee54cf88329: [Hexagon] Account for truncating pairs to non-pairs when widening truncates.

Just llc -march=hexagon < bad_hvx64_code.ll, but I added "target-cpu"="hexagonv65" "target-features"="+hvx,+hvx-length64b" to attributes inside the file.

Sep 15 2020, 10:35 AM
srj added a comment to rG0ee54cf88329: [Hexagon] Account for truncating pairs to non-pairs when widening truncates.

The llc' from 621b10ca187bdd6de691338e48b288ea1c6a5822, generates the same code as llc` from 0ee54cf88329c50f25872ac1c67d7ae60ee3154c for this file. Are there additional optimizations happening on this bitcode before it goes to codegen?

Sep 15 2020, 10:18 AM
srj added a comment to D87379: [ARM] Selects SSAT/USAT from LLVM IR of min/max patterns.

It seems we made a small mistake in one of the if conditions, will fix it now. Thanks for letting us know

Sep 15 2020, 9:12 AM · Restricted Project

Sep 14 2020

srj added a comment to rG0ee54cf88329: [Hexagon] Account for truncating pairs to non-pairs when widening truncates.

Great, thanks! Will take a look tomorrow.

Sep 14 2020, 5:11 PM
srj added a comment to rG0ee54cf88329: [Hexagon] Account for truncating pairs to non-pairs when widening truncates.

Here's some Halide-generated llvm assembly that shows the code pattern that is the issue; executing this code (via the Qualcomm HVX emulator) works fine when this is compiled with LLVM @ commit 621b10ca187, but gets incorrect results as of commit 0ee54cf8832. (Note that while I have tried to minimize unrelated runtime code in this file, there is some that is hard to eliminate; rather than trying to edit it, I recommend you skip to the function named bad_hvx64_code.)

Sep 14 2020, 4:21 PM
srj added a comment to rG0ee54cf88329: [Hexagon] Account for truncating pairs to non-pairs when widening truncates.

Do you know what vector types are involved (e.g. <32 x i8>)? Anything with i1 or i64?

Sep 14 2020, 3:49 PM
srj added a comment to rG0ee54cf88329: [Hexagon] Account for truncating pairs to non-pairs when widening truncates.

Sure. Keep me posted.

Sep 14 2020, 12:00 PM

Sep 11 2020

srj added a comment to rG0ee54cf88329: [Hexagon] Account for truncating pairs to non-pairs when widening truncates.

So after further investigation, it appears that I was wrong about this completely fixing the issue that Halide was seeing from https://reviews.llvm.org/rG1387f96ab3310678df62c1073346ca387a85f656 . (I neglected to force clean rebuilds of LLVM while testing, which turned out to be a false economy in terms of time savings.)

Sep 11 2020, 4:41 PM

Sep 9 2020

srj added a comment to rG0ee54cf88329: [Hexagon] Account for truncating pairs to non-pairs when widening truncates.

Appears to fix our issue. Thanks so much for such a speedy turnaround!

Sep 9 2020, 12:50 PM
srj added a comment to rG0ee54cf88329: [Hexagon] Account for truncating pairs to non-pairs when widening truncates.

Testing now.

Sep 9 2020, 12:38 PM
srj added a comment to rG1387f96ab331: [Hexagon] Handle widening of vector truncate.

Unfortunately, I cannot attach a bitcode or similar file to replicate this, as the crash occurs during the phase that would produce bitcode.

Sep 9 2020, 11:56 AM
srj added a comment to rG1387f96ab331: [Hexagon] Handle widening of vector truncate.

This appears to have injected a failure in Halide's tests for hvx64 codegen; a test that exercises predicated store-load operations now fails for hvx64 (but not hvx128) with

Sep 9 2020, 11:46 AM

Aug 18 2020

srj added a comment to D86173: Don't look for terminfo libs when LLVM_ENABLE_TERMINFO=OFF.

Thanks. this fixes the issues I had with https://reviews.llvm.org/D85820 and Halide. LGTM.

Aug 18 2020, 4:38 PM · Restricted Project, Restricted Project
srj added inline comments to D85820: Use find_library for ncurses.
Aug 18 2020, 2:42 PM · Restricted Project, Restricted Project, Restricted Project
srj added inline comments to D85820: Use find_library for ncurses.
Aug 18 2020, 2:37 PM · Restricted Project, Restricted Project, Restricted Project
srj added a comment to D85820: Use find_library for ncurses.

This seems to have injected a link-time dependency on setupterm etc. even if you configure CMake with LLVM_ENABLE_TERMINFO=OFF (which was not the case before) -- this is breaking some builds of Halide as a result. (Should the #idef LLVM_ENABLE_TERMINFO actually if #if instead of #ifdef?)

Aug 18 2020, 2:26 PM · Restricted Project, Restricted Project, Restricted Project

Aug 11 2020

srj added a comment to D81766: [VectorCombine] try to create vector loads from scalar loads.

Can we just add another condition (!VectorSize) to this bailout?
if (!ScalarSize || VectorSize % ScalarSize != 0)

Aug 11 2020, 5:17 PM · Restricted Project
srj added a comment to D81766: [VectorCombine] try to create vector loads from scalar loads.

Update: it appears that VectorSize (from TTI.getMinVectorRegisterBitWidth()) is zero in this case, which causes the assertion failure.

Aug 11 2020, 5:05 PM · Restricted Project
srj added a comment to D81766: [VectorCombine] try to create vector loads from scalar loads.

This appears to have broken some of Halide's codegen for Hexagon/HVX; as of this revision, some of our tests are now failing with

Aug 11 2020, 3:57 PM · Restricted Project

Jun 4 2020

srj added a comment to D81013: [InstCombine] move vector select ahead of select-shuffle.

@spatel thanks for the quick fix!

Jun 4 2020, 3:30 PM · Restricted Project
srj added a comment to D81013: [InstCombine] move vector select ahead of select-shuffle.

This seems to have injected breakages into Halide when used for x86-64 targets with 'narrow' SIMD (eg., using only SSE2 or SSE4.1); many of our tests now fail in that configuration with variations of

Jun 4 2020, 1:15 PM · Restricted Project

May 26 2020

srj added a comment to D50078: clang-format: support aligned nested conditionals formatting.

And I also think it should be on by default instead of modifying many .clang-format files. So IMHO if we add an option, it should be opt-out.

May 26 2020, 10:16 AM · Restricted Project, Restricted Project

May 21 2020

srj added a comment to D50078: clang-format: support aligned nested conditionals formatting.

I think the new formatting is unquestionably better than the old formatting.

May 21 2020, 4:49 PM · Restricted Project, Restricted Project

Mar 12 2020

srj added a comment to rG8ec71585719d: [InstSimplify] simplify FP ops harder with FMF.

This seems to have injected a breakage into Halide; as of this change, one of our tests (a vectorized reduction of a bunch of infinities to find the minimum, which should also be infinity) now fails with the result being an apparently-garbage value that varies from run to run.

Mar 12 2020, 4:51 PM
srj added a comment to rG8ec71585719d: [InstSimplify] simplify FP ops harder with FMF.

This seems to have injected a breakage into Halide; as of this change, one of our tests (a vectorized reduction of a bunch of infinities to find the minimum, which should also be infinity) now fails with the result being an apparently-garbage value that varies from run to run.

Mar 12 2020, 4:51 PM

Feb 6 2020

srj added a comment to D74102: [CMake] Link against ZLIB::ZLIB.

We too get this bogus result from llvm-config (and it's breaking some of our builds). Please fix or revert.

Feb 6 2020, 1:29 PM · Restricted Project

Dec 2 2019

srj added a comment to rG340e7c0b77a7: build: avoid hardcoding the libxml2 library name.

This has injected a bug into llvm-config --system-libs (at least, on OSX and Linux) for me; previously, the output would be something like

Dec 2 2019, 12:06 PM
srj added a comment to D69412: build: avoid hardcoding the libxml2 library name.

I haven't found any reasonable workarounds for this breakage; CMake is apparently splitting -l/usr/lib/x86_64-linux-gnu/libxml2.so into -l/usr/lib/x86_64 and -linux-gnu/libxml2.so, neither of which are valid dependencies, so our builds are totally broken. If a fix isn't forthcoming, can we please roll this change back at your earliest convenience?

Dec 2 2019, 11:38 AM · Restricted Project

Dec 1 2019

srj added a comment to D69412: build: avoid hardcoding the libxml2 library name.

This has injected a bug into llvm-config --system-libs (at least, on OSX and Linux) for me; previously, the output would be something like

Dec 1 2019, 5:40 PM · Restricted Project

Jul 23 2019

srj added a comment to rL366860: FileCheck [8/12]: Define numeric var from expr.

If I change line 345 to explicitly move-and-construct the result, it builds fine:

Jul 23 2019, 5:54 PM
srj added a comment to rL366860: FileCheck [8/12]: Define numeric var from expr.

This commit is failing to compile for me (on Linux x86-64, gcc 7.3):

Jul 23 2019, 5:28 PM

Aug 24 2018

srj added a comment to D46179: [X86] Lowering addus/subus intrinsics to native IR (LLVM part).

Would it help or hurt if we narrowed the select in IR to match the final return type and original operand types (I made the types smaller from your code just to make this easier to read):

Aug 24 2018, 11:01 AM
srj added a comment to D46179: [X86] Lowering addus/subus intrinsics to native IR (LLVM part).

I took the liberty of opening a bug on this to simplify discussion: https://bugs.llvm.org/show_bug.cgi?id=38691

Aug 24 2018, 10:54 AM

Aug 23 2018

srj added a comment to D46179: [X86] Lowering addus/subus intrinsics to native IR (LLVM part).

I'm trying to update Halide to generate IR that will be recognized by this patch (instead of calling the now-deprecated intrinsics), but having trouble with a somewhat degenerate-but-legal case.

Aug 23 2018, 6:39 PM