Page MenuHomePhabricator

srj (Steven Johnson)
User

Projects

User does not belong to any projects.

User Details

User Since
Aug 23 2018, 5:53 PM (146 w, 2 d)

Recent Activity

Apr 26 2021

srj added a comment to D100492: [OpenCL] Change OpenCL builtin version encoding.

I am happy to make/accept any adjustments to this patch to fix the particular build. I don't have access to MSVC though, would you be able to help figure out what the problematic construct in my patch was?

Apr 26 2021, 10:52 AM · Restricted Project
srj added a comment to D100492: [OpenCL] Change OpenCL builtin version encoding.

(This is clearly just a weird bug on MSVC's part; there's nothing about the code here that seems obviously unreasonable or complex. Working around compiler bugs is a thing, alas.)

Apr 26 2021, 10:27 AM · Restricted Project
srj added a comment to D100492: [OpenCL] Change OpenCL builtin version encoding.

From experimentation, it appears that just pulling the MinVersion and MaxVersion expressions from BuiltinNameEmitter::EmitBuiltinTable into separate statements will pacify MSVC, e.g.

Apr 26 2021, 10:25 AM · Restricted Project
srj added a comment to D100492: [OpenCL] Change OpenCL builtin version encoding.

This change appears to be the injection point for https://bugs.llvm.org/show_bug.cgi?id=50128 -- trying to build clang-tblgen after this change crashes VC2019 if targeting 32-bit Windows (see below). While this is likely an MSVC bug, I presume we still want to support crosscompiling for 32-bit Windows?

Apr 26 2021, 10:15 AM · Restricted Project

Apr 22 2021

srj added a comment to D100981: Delete le32/le64 targets.

Any chance that this could be (temporarily) reverted? This will break literally all Halide compilation; we're working on a fix on our side, but it would be nice to have a few days to be sure we have it right, and I suspect there's no urgency to removing this right now.

Will https://github.com/halide/Halide/pull/5934 take longer? If it does, I can temporarily revert this.

Apr 22 2021, 10:05 AM · Restricted Project, Restricted Project
srj added a comment to D100981: Delete le32/le64 targets.

Any chance that this could be (temporarily) reverted? This will break literally all Halide compilation; we're working on a fix on our side, but it would be nice to have a few days to be sure we have it right, and I suspect there's no urgency to removing this right now.

Apr 22 2021, 9:28 AM · Restricted Project, Restricted Project

Apr 21 2021

srj added a comment to D100099: [X86][CostModel] Try to fix cost computation load/stores of non-power-of-two vectors.

Commit a511b55cfd67acecc58f1ccf1f3ce5c917dc1d90 fixes both the llc hang and MCJIT-specific hang. Thanks for the attention.

Apr 21 2021, 12:07 PM · Restricted Project
srj added a comment to D100099: [X86][CostModel] Try to fix cost computation load/stores of non-power-of-two vectors.

If it hangs all the way back, then the bisection *can not* point to this revision, and this could not have triggered/exposed this.

Apr 21 2021, 10:27 AM · Restricted Project

Apr 20 2021

srj added a comment to D100099: [X86][CostModel] Try to fix cost computation load/stores of non-power-of-two vectors.

FYI, I've opened a bug (https://bugs.llvm.org/show_bug.cgi?id=50049) for the llc hang, as it seems like a clear problem regardless of whether this commit is involved.

Apr 20 2021, 4:20 PM · Restricted Project
srj added a comment to D100099: [X86][CostModel] Try to fix cost computation load/stores of non-power-of-two vectors.

Update:

  • rerunning bisect still points at this commit.
  • it looks like the reproducer I uploaded does indeed hang llc quite a ways back -- even llc from an LLVM12 build hangs in the same way.
  • running llc at -O0 succeeds, but -O1 or higher still fails.
  • Looks like we're triggering exponential runtime somewhere under TargetLowering::SimplifyDemandedBits.
Apr 20 2021, 4:02 PM · Restricted Project
srj added a comment to D100099: [X86][CostModel] Try to fix cost computation load/stores of non-power-of-two vectors.

Please rebisect, reverting this doesn't make that hang go away for me.

Apr 20 2021, 3:40 PM · Restricted Project
srj added a comment to D100099: [X86][CostModel] Try to fix cost computation load/stores of non-power-of-two vectors.

Please rebisect, reverting this doesn't make that hang go away for me.

Apr 20 2021, 3:09 PM · Restricted Project
srj added a comment to D100099: [X86][CostModel] Try to fix cost computation load/stores of non-power-of-two vectors.

OK, here's a file that I think will repro it:

Apr 20 2021, 2:21 PM · Restricted Project
srj added a comment to D100099: [X86][CostModel] Try to fix cost computation load/stores of non-power-of-two vectors.

Hm, interesting. Could you please also check if that still reproduces with D100684 (awaiting review) ?

Testing now

Apr 20 2021, 2:12 PM · Restricted Project
srj added a comment to D100099: [X86][CostModel] Try to fix cost computation load/stores of non-power-of-two vectors.

Hm, interesting. Could you please also check if that still reproduces with D100684 (awaiting review) ?

Apr 20 2021, 2:11 PM · Restricted Project
srj added a comment to D100099: [X86][CostModel] Try to fix cost computation load/stores of non-power-of-two vectors.

This commit appears to have injected a hang (or > 60s delay) in certain Halide tests when using the JIT (the "hang" being inside LLVM code, but JIT-generated code). I'm working on finding a small repro case.

Apr 20 2021, 1:47 PM · Restricted Project

Apr 7 2021

srj added a comment to D100018: [WebAssembly] Add shuffles as an option for lowering BUILD_VECTOR.

This definitely improves codegen for Halide substantially.

Apr 7 2021, 2:20 PM · Restricted Project

Jan 13 2021

srj added a comment to D94607: [Hexagon] Improve legalizing of ISD::SETCC result.

This appears to fix the issues that Halide was having. Please land at your earliest convenience :-)

Jan 13 2021, 10:21 AM · Restricted Project
srj added a comment to rGa90214760d04: [Hexagon] Custom-widen SETCC's operands.

Could you try this patch? https://reviews.llvm.org/D94607

Jan 13 2021, 10:21 AM
srj updated subscribers of rGa90214760d04: [Hexagon] Custom-widen SETCC's operands.

Thanks, testing now.

Jan 13 2021, 9:44 AM

Jan 12 2021

srj added a comment to rGa90214760d04: [Hexagon] Custom-widen SETCC's operands.

Going further, it seems that VecWidth is 16 in this case -- it's not at all clear to me why this worked before but is failing now.

Jan 12 2021, 5:29 PM
srj added a comment to rGa90214760d04: [Hexagon] Custom-widen SETCC's operands.

This has injected a failure into Halide's HVX codegen; running a test that generates various dense load/stores, we now fail at compile time with an assertion inside LLVM: llvm::SDValue llvm::HexagonTargetLowering::extractVector(llvm::SDValue, llvm::SDValue, const llvm::SDLoc&, llvm::MVT, llvm::MVT, llvm::SelectionDAG&) const: Assertion VecWidth == 8 || VecWidth == 4 || VecWidth == 2' failed.`

Jan 12 2021, 5:22 PM

Oct 13 2020

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.

Oct 13 2020, 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