Page MenuHomePhabricator

Please use GitHub pull requests for new patches. Phabricator shutdown timeline

gflegar (Goran Flegar)
User

Projects

User does not belong to any projects.

User Details

User Since
Apr 29 2022, 12:34 AM (74 w, 1 d)

Recent Activity

Thu, Sep 28

gflegar committed rG042468bff5ae: [mlir][SME] Fix unused variable warning (authored by gflegar).
[mlir][SME] Fix unused variable warning
Thu, Sep 28, 6:24 AM · Restricted Project, Restricted Project

Aug 1 2023

gflegar accepted D156714: Released restriction that prevented implicit dynamic-to-static dimension type cast in TOSA ops..
Aug 1 2023, 7:24 AM · Restricted Project, Restricted Project

Jul 25 2023

gflegar committed rG118f95b39465: [bazel][mlir] Port ff8775f3fffea787922eb238c0af3ed5715485d9 (authored by gflegar).
[bazel][mlir] Port ff8775f3fffea787922eb238c0af3ed5715485d9
Jul 25 2023, 11:00 AM · Restricted Project
gflegar committed rG179d2efce6de: [bazel][mlir] Port ca9a3354d04b15366088d7831b40f891e3d77b95 (authored by gflegar).
[bazel][mlir] Port ca9a3354d04b15366088d7831b40f891e3d77b95
Jul 25 2023, 2:45 AM · Restricted Project

Jul 11 2023

gflegar accepted D154976: Fix bazel build file for D154060..
Jul 11 2023, 8:32 AM · Restricted Project, Restricted Project

May 19 2023

gflegar committed rGffd9d6e0ed89: [mlir][linalg] Fix unused variable on opt build (authored by gflegar).
[mlir][linalg] Fix unused variable on opt build
May 19 2023, 5:02 AM · Restricted Project, Restricted Project

May 16 2023

gflegar committed rG2f4c96097a3c: [bazel] Fix build after 0c4d7d14e94d (authored by gflegar).
[bazel] Fix build after 0c4d7d14e94d
May 16 2023, 9:28 AM · Restricted Project

Mar 7 2023

gflegar accepted D145398: Make mlir-opt --show-dialects option print on a single line.
Mar 7 2023, 1:17 AM · Restricted Project, Restricted Project

Mar 6 2023

gflegar added a comment to D144782: Expose a convenient registerCLOptions() for MlirOptMainConfig.

Your commit above is just not warranted IMO as it does not fix any upstream problem.

Mar 6 2023, 11:04 AM · Restricted Project, Restricted Project
gflegar added inline comments to D145398: Make mlir-opt --show-dialects option print on a single line.
Mar 6 2023, 10:38 AM · Restricted Project, Restricted Project
gflegar added a comment to D144782: Expose a convenient registerCLOptions() for MlirOptMainConfig.

Submitted https://github.com/llvm/llvm-project/commit/f2cdccc034d0696f186f55d6826dd6ac40e4d3d5 for now that restores the preloading behavior, and fixes the test that was failing in our internal configuration. If we really need to support running commands that expect to be given an EOF input to work, we can work on our internal lit runner to support that, but I would prefer to have a heads-up for that before.

Mar 6 2023, 10:36 AM · Restricted Project, Restricted Project
gflegar committed rGf2cdccc034d0: [mlir-opt] Fix dialect preload after fb1bb6a (authored by gflegar).
[mlir-opt] Fix dialect preload after fb1bb6a
Mar 6 2023, 10:32 AM · Restricted Project, Restricted Project
gflegar added inline comments to D145398: Make mlir-opt --show-dialects option print on a single line.
Mar 6 2023, 10:21 AM · Restricted Project, Restricted Project
gflegar added a comment to D144782: Expose a convenient registerCLOptions() for MlirOptMainConfig.

Yes, "registered" is what I meant.

Mar 6 2023, 10:00 AM · Restricted Project, Restricted Project
gflegar added a comment to D144782: Expose a convenient registerCLOptions() for MlirOptMainConfig.

Right, any convenient ways to just get a list of loaded dialects from a script. mlir-opt --show-dialects --version doesn't work (I guess version is processed before we get to --show-dialects), and echo "" | mlir-opt --show-dialects is a weird thing to have.

Mar 6 2023, 9:12 AM · Restricted Project, Restricted Project
gflegar added a comment to D144782: Expose a convenient registerCLOptions() for MlirOptMainConfig.

Ah, the --show-dialects option is no longer a stand-alone thing, but an additional printout, where you could also do other things. So I guess in my example it's still waiting for some IR as input.

Mar 6 2023, 9:00 AM · Restricted Project, Restricted Project
gflegar added a comment to D144782: Expose a convenient registerCLOptions() for MlirOptMainConfig.

This seems to cause mlir-opt to hang when passed the --show-dialects option. Ran it through bazel:

Mar 6 2023, 8:57 AM · Restricted Project, Restricted Project
gflegar committed rGd866f87f88bb: [bazel] Fix build after 28d04c5 (authored by gflegar).
[bazel] Fix build after 28d04c5
Mar 6 2023, 8:42 AM · Restricted Project

Feb 3 2023

gflegar added inline comments to D143064: [mlir][llvm] Add structured loop metadata.
Feb 3 2023, 5:03 AM · Restricted Project, Restricted Project
gflegar added inline comments to D143064: [mlir][llvm] Add structured loop metadata.
Feb 3 2023, 4:59 AM · Restricted Project, Restricted Project

Jan 31 2023

gflegar committed rG5bc4e1cd3be8: [bazel] Add missing includes for 5212058 (authored by gflegar).
[bazel] Add missing includes for 5212058
Jan 31 2023, 2:55 AM · Restricted Project

Jan 17 2023

gflegar added a comment to D141804: [mlir][linalg] Omit printing result types for named ops..

From my experience integrating MLIR into Google's internal repository, breakages from changes like these seem to be very frequent (e.g., the two already mentioned above), so I don't see why this change should be blocked while others get a pass. If we want to be more careful about API breakages, then we should first formalize this somehow and apply the same rules to everyone.
I never considered the printed version of MLIR stable, and I'm not sure that its stability should be the goal that prevents improvements to the printing format.
Regarding churn, we should not forget that not improving the printed format also causes churn every time someone is reading or modifying it.
I think that redundancy in IR causes more issues than it solves, so I would prefer landing this change.

Jan 17 2023, 2:43 AM · Restricted Project, Restricted Project

Jan 12 2023

gflegar committed rGb7fe0d346b30: Lower math.tan to __nv_tan[f] / __ocml_tan_f{32|64} (authored by gflegar).
Lower math.tan to __nv_tan[f] / __ocml_tan_f{32|64}
Jan 12 2023, 6:27 AM · Restricted Project, Restricted Project
gflegar closed D141505: Lower math.tan to __nv_tan[f] / __ocml_tan_f{32|64}.
Jan 12 2023, 6:27 AM · Restricted Project, Restricted Project

Jan 11 2023

gflegar updated the summary of D141505: Lower math.tan to __nv_tan[f] / __ocml_tan_f{32|64}.
Jan 11 2023, 10:40 AM · Restricted Project, Restricted Project
gflegar retitled D141505: Lower math.tan to __nv_tan[f] / __ocml_tan_f{32|64} from Lower math.tan to __nv_tan[f] to Lower math.tan to __nv_tan[f] / __ocml_tan_f{32|64}.
Jan 11 2023, 10:40 AM · Restricted Project, Restricted Project
gflegar updated the diff for D141505: Lower math.tan to __nv_tan[f] / __ocml_tan_f{32|64}.

Also lower math.tan for ROCDL

Jan 11 2023, 10:39 AM · Restricted Project, Restricted Project
gflegar requested review of D141505: Lower math.tan to __nv_tan[f] / __ocml_tan_f{32|64}.
Jan 11 2023, 7:43 AM · Restricted Project, Restricted Project

Jan 10 2023

gflegar added a comment to D137786: Lower arith.min/max to llvm.intr.minimum/maximum.

I don't think we even have a legalization implementation to expand these. You almost certainly intended to use llvm.minnum/maxnum

Jan 10 2023, 5:15 AM · Restricted Project, Restricted Project

Nov 10 2022

gflegar added a comment to D137655: Expand fminimum/fmaximum into fminnum/fmaxnum + NaN check.

I don't think this is right either. LLVM defines minnum according to the old semantics, which don't specify an order between zeroes. We'd need a separate ISD opcode for minnum according to 2018 semantics.

Interestingly though, for the NVPTX backend this will end up producing the correct code, since minnum is lowered to the min/max PTX instructions, which defines the 2018 semantics. (I do agree though that the intermediate code is not correct.)

It's not OK to have wrong intermediate code. We do have the "new" semantic opcodes already in FMINIMUM/FMAXIMUM

It's even less OK to fail altogether (which is what is happening without this patch). And we're not talking about the new semantics for FMINIMUM/FMAXIMUM, but for the new semantics of FMINNUM/FMAXNUM (the +/-0 handling changed).

Nov 10 2022, 9:14 AM · Restricted Project, Restricted Project
gflegar updated the diff for D137655: Expand fminimum/fmaximum into fminnum/fmaxnum + NaN check.

Formatting fixes

Nov 10 2022, 9:10 AM · Restricted Project, Restricted Project
gflegar requested review of D137786: Lower arith.min/max to llvm.intr.minimum/maximum.
Nov 10 2022, 7:03 AM · Restricted Project, Restricted Project
gflegar retitled D137655: Expand fminimum/fmaximum into fminnum/fmaxnum + NaN check from Expand fminimum and fmaximum into a pair of selects to Expand fminimum/fmaximum into fminnum/fmaxnum + NaN check.
Nov 10 2022, 6:49 AM · Restricted Project, Restricted Project
gflegar added a comment to D137655: Expand fminimum/fmaximum into fminnum/fmaxnum + NaN check.

I think this is the best we can do short of adding a new op. We only do the expansion for the NVPTX backed, and don't support it otherwise. In the NVPTX backed, the intermediate code still ends up being semantically incorrect for +/-0.0, but since FMINNUM/FMAXNUM lower to PTX min/max, which do implement the 2018 semantics of those ops, the final PTX ends up being correct.

Nov 10 2022, 6:48 AM · Restricted Project, Restricted Project
gflegar updated the diff for D137655: Expand fminimum/fmaximum into fminnum/fmaxnum + NaN check.

Limit expansion only to NVPTX backend

Nov 10 2022, 6:43 AM · Restricted Project, Restricted Project
gflegar added a comment to D137655: Expand fminimum/fmaximum into fminnum/fmaxnum + NaN check.

I don't think this is right either. LLVM defines minnum according to the old semantics, which don't specify an order between zeroes. We'd need a separate ISD opcode for minnum according to 2018 semantics.

Nov 10 2022, 1:46 AM · Restricted Project, Restricted Project
gflegar added a comment to D137655: Expand fminimum/fmaximum into fminnum/fmaxnum + NaN check.

I don't think this is right either. LLVM defines minnum according to the old semantics, which don't specify an order between zeroes. We'd need a separate ISD opcode for minnum according to 2018 semantics.

Unfortunately, I don't have the bandwidth to chase this rabbit hole further, especially since our use case is insensitive to what happens for -0 and +0. I can add a TODO comment to fix this. Though I would still argue for submitting this, as it is correct modulo -0/+0, which is far preferable to the current state where we have a silent failure (and produce invalid code) for the backends that attempt to expand the op (like in NVPTX).

That sounds like an unrelated bug. If an operation is Expand, but we don't support expanding it, shouldn't that result in an isel failure?

Nov 10 2022, 1:40 AM · Restricted Project, Restricted Project

Nov 9 2022

gflegar added a comment to D137655: Expand fminimum/fmaximum into fminnum/fmaxnum + NaN check.

I don't think this is right either. LLVM defines minnum according to the old semantics, which don't specify an order between zeroes. We'd need a separate ISD opcode for minnum according to 2018 semantics.

Nov 9 2022, 8:12 AM · Restricted Project, Restricted Project
gflegar added a comment to D137655: Expand fminimum/fmaximum into fminnum/fmaxnum + NaN check.

@nikic 0 handling should be fixed now

Nov 9 2022, 7:23 AM · Restricted Project, Restricted Project
gflegar updated the diff for D137655: Expand fminimum/fmaximum into fminnum/fmaxnum + NaN check.

Change lowering to minnum/maxnum + NaN check

Nov 9 2022, 7:20 AM · Restricted Project, Restricted Project
gflegar added a comment to D137655: Expand fminimum/fmaximum into fminnum/fmaxnum + NaN check.

Doesn't this handle signed zero incorrectly?

I believe it is:

For FMINIMUM:

Tmp1 = -0.0, Tmp2 = +0.0  =>  Tmp1 ULT Tmp2 == True   =>  Tmp3 = Tmp1 = -0.0;     Tmp2 UO Tmp2 = False  =>  Result = Tmp3 = -0.0

Isn't -0.0 ULT 0.0 false, because negative zero and positive zero are equal?

Nov 9 2022, 5:00 AM · Restricted Project, Restricted Project
gflegar added a comment to D137655: Expand fminimum/fmaximum into fminnum/fmaxnum + NaN check.

Doesn't this handle signed zero incorrectly?

Nov 9 2022, 2:16 AM · Restricted Project, Restricted Project

Nov 8 2022

gflegar added a reviewer for D137655: Expand fminimum/fmaximum into fminnum/fmaxnum + NaN check: csigg.
Nov 8 2022, 9:53 AM · Restricted Project, Restricted Project
gflegar requested review of D137655: Expand fminimum/fmaximum into fminnum/fmaxnum + NaN check.
Nov 8 2022, 9:51 AM · Restricted Project, Restricted Project

Oct 10 2022

gflegar committed rGc7545de9b4b7: [mlir][Bazel] Fix for reviews.llvm.org/D135559 (authored by gflegar).
[mlir][Bazel] Fix for reviews.llvm.org/D135559
Oct 10 2022, 7:29 AM · Restricted Project

Aug 22 2022

gflegar committed rG59548fe873d8: [mlir] Fix compile errors with bytecode support (authored by gflegar).
[mlir] Fix compile errors with bytecode support
Aug 22 2022, 9:01 AM · Restricted Project, Restricted Project
gflegar committed rG1d9b1427f4ea: [mlir][Bazel] Fix bazel build (authored by gflegar).
[mlir][Bazel] Fix bazel build
Aug 22 2022, 3:09 AM · Restricted Project

May 31 2022

gflegar abandoned D126310: Do not destroy attrs in MergeNestedParallelLoops.

We can achieve what we need by adding LoopInvariantCodeMotionPass. Abandoning this.

May 31 2022, 7:20 AM · Restricted Project, Restricted Project
gflegar added a comment to D126310: Do not destroy attrs in MergeNestedParallelLoops.

In essence, the issue is that tiling, mapping to gpu and the transformation to gpu dialect have to run after each other without being disturbed by other passes.

May 31 2022, 5:14 AM · Restricted Project, Restricted Project

May 30 2022

gflegar added inline comments to D126310: Do not destroy attrs in MergeNestedParallelLoops.
May 30 2022, 10:58 AM · Restricted Project, Restricted Project
gflegar added inline comments to D126310: Do not destroy attrs in MergeNestedParallelLoops.
May 30 2022, 9:50 AM · Restricted Project, Restricted Project
gflegar added a reviewer for D126310: Do not destroy attrs in MergeNestedParallelLoops: herhut.
May 30 2022, 9:40 AM · Restricted Project, Restricted Project
gflegar added a comment to D126310: Do not destroy attrs in MergeNestedParallelLoops.

Worked like a charm.

May 30 2022, 4:46 AM · Restricted Project, Restricted Project
gflegar retitled D126310: Do not destroy attrs in MergeNestedParallelLoops from Label MergeNestedParallelLoops patterns to Do not destroy attrs in MergeNestedParallelLoops.
May 30 2022, 4:42 AM · Restricted Project, Restricted Project
gflegar updated the diff for D126310: Do not destroy attrs in MergeNestedParallelLoops.

Prevent merging loops with attrs instead

May 30 2022, 4:33 AM · Restricted Project, Restricted Project
gflegar added a comment to D126310: Do not destroy attrs in MergeNestedParallelLoops.

Sure, I can change to addWithLabel instead.

May 30 2022, 3:02 AM · Restricted Project, Restricted Project

May 24 2022

gflegar added a reviewer for D126310: Do not destroy attrs in MergeNestedParallelLoops: csigg.
May 24 2022, 10:59 AM · Restricted Project, Restricted Project
gflegar requested review of D126310: Do not destroy attrs in MergeNestedParallelLoops.
May 24 2022, 10:58 AM · Restricted Project, Restricted Project

May 3 2022

gflegar committed rG672b908bca67: [mlir] Add sin & cos ops to complex dialect (authored by gflegar).
[mlir] Add sin & cos ops to complex dialect
May 3 2022, 10:37 AM · Restricted Project, Restricted Project
gflegar closed D124773: [mlir] Add sin & cos ops to complex dialect.
May 3 2022, 10:37 AM · Restricted Project, Restricted Project
gflegar updated the diff for D124773: [mlir] Add sin & cos ops to complex dialect.

Minor formatting

May 3 2022, 10:27 AM · Restricted Project, Restricted Project
gflegar added inline comments to D124773: [mlir] Add sin & cos ops to complex dialect.
May 3 2022, 10:22 AM · Restricted Project, Restricted Project
gflegar updated the diff for D124773: [mlir] Add sin & cos ops to complex dialect.

Make combine a regular function, use CHECK-DAG, formatting

May 3 2022, 9:53 AM · Restricted Project, Restricted Project
gflegar added inline comments to D124773: [mlir] Add sin & cos ops to complex dialect.
May 3 2022, 9:21 AM · Restricted Project, Restricted Project
gflegar added inline comments to D124773: [mlir] Add sin & cos ops to complex dialect.
May 3 2022, 9:20 AM · Restricted Project, Restricted Project
gflegar added inline comments to D124773: [mlir] Add sin & cos ops to complex dialect.
May 3 2022, 9:15 AM · Restricted Project, Restricted Project
gflegar added inline comments to D124773: [mlir] Add sin & cos ops to complex dialect.
May 3 2022, 8:46 AM · Restricted Project, Restricted Project

May 2 2022

gflegar added a reviewer for D124773: [mlir] Add sin & cos ops to complex dialect: frgossen.
May 2 2022, 7:57 AM · Restricted Project, Restricted Project
gflegar requested review of D124773: [mlir] Add sin & cos ops to complex dialect.
May 2 2022, 7:56 AM · Restricted Project, Restricted Project