Integrate llvm-project and mlir-hlo.

[stellaraccident]

Corresponding commits:

• mlir-hlo: 16886a108eff5197f816ca0f1950cc5ff1b078d9
• stablehlo: 77a59815a82b34f7b08ed2d42a711d9920682d0e
• llvm-project: 4acc3ff

[ramiro050]

Hi @stellaraccident, would it be possible to include the commits in #1178 (comment) as part of this bump?

There's a team that uses torch-mlir and onnx-mlir, and the commits from last week are the ones that align with onnx-mlir's bi-weekly bump (brought up during the developer hour this morning).

[stellaraccident]

Hi @stellaraccident, would it be possible to include the commits in #1178 (comment) as part of this bump?

There's a team that uses torch-mlir and onnx-mlir, and the commits from last week are the ones that align with onnx-mlir's bi-weekly bump (brought up during the developer hour this morning).

It's a week behind and I read that as being unmaintained now. Sorry I didn't make it to the developer hour: have we gotten a volunteer yet to help with this (I didn't see any comments to my request)?

[stellaraccident]

Will iterate on this some more now that compile breaks are cleared. I think something got messed up in the region branch thing.

[ramiro050]

It's a week behind and I read that as being unmaintained now.

Yeah, the person assigned had quite a few things on their plate, so the bump didn't get made. Normally someone would ping the person to check on the status, but I think we were all a bit distracted last week.

Is it possible to have two commits on this PR so that the people using onnx-mlir don't have to wait over a week to get a matching llvm?

cc: @philass

have we gotten a volunteer yet to help with this (I didn't see any comments to my request)?

I have pinged the main contributors of the stablehlo backend in torch-mlir to take a look at your proposal, since I imagine it affects them the most. I think they missed the notification from last week.

[stellaraccident]

It's a week behind and I read that as being unmaintained now.

Yeah, the person assigned had quite a few things on their plate, so the bump didn't get made. Normally someone would ping the person to check on the status, but I think we were all a bit distracted last week.

Is it possible to have two commits on this PR so that the people using onnx-mlir don't have to wait over a week to get a matching llvm?

cc: @philass

have we gotten a volunteer yet to help with this (I didn't see any comments to my request)?

I have pinged the main contributors of the stablehlo backend in torch-mlir to take a look at your proposal, since I imagine it affects them the most. I think they missed the notification from last week.

I'm going to keep working on getting this patch working. If there is a volunteer, they can rebase, but we have to keep the train moving.

[ramiro050]

I'm going to keep working on getting this patch working. If there is a volunteer, they can rebase, but we have to keep the train moving.

I'm happy to do the rebase 🙂

[philass]

@ramiro050, @stellaraccident,

Thanks for the consideration of our situation during the uplift. Feel free to skip the intermediate LLVM uplifts to match the onnx-mlir llvm commit.

As long as we get the weekly schedule back on track and the LLVM commit gets bumped next Monday after this lands, we shouldn't have any problems.

[stellaraccident]

The random PyTorch crashes I was chasing yesterday with this bump disappeared overnight/with restart of the CI runners. Was also experiencing this locally but suspect maybe ccache related. In either case, just down to two tosa failures. Can see about patching those and landing this morning. @eric-k256 in case if there is any context.

[eric-k256]

We updated the tosa printers in LLVM, it looks like that is included in the bump. #2345 has the updated lit tests to match the LLVM change. I'm not sure what the easiest way is to integrate that PR with the LLVM bump.

[stellaraccident]

We updated the tosa printers in LLVM, it looks like that is included in the bump. #2345 has the updated lit tests to match the LLVM change. I'm not sure what the easiest way is to integrate that PR with the LLVM bump.

Thanks - I cherry-picked that commit into this branch. Will see what the CI says.

Would you mind having a look/approving? If the CI is clean, I'll land it.

[stellaraccident]

@eric-k256 There is a legit lowering failure: https://github.com/llvm/torch-mlir/actions/runs/6162809713/job/16725038984?pr=2454

If you have a pointer, I can try to patch but would be best if someone familiar with that part triaged. Feel free to push a fix to this branch (or another).

[eric-k256]

I'll take a look and see what's going on. (I was just about to approve assuming CI would pass)

[stellaraccident]

I'll take a look and see what's going on. (I was just about to approve assuming CI would pass)

If you've got a pointer, I can make a change quickly... just not in a "deep thinking" about it state for the next hour or so.

[ramiro050]

Relevant patch: https://reviews.llvm.org/D157424

We need to update the TorchToTosa lowerings to cast the index into the an i32

[ramiro050]

Might require a few more things, but I think replacing getI64IntegerAttr with the right attribute getter in the file:

https://github.com/llvm/torch-mlir/blob/main/lib/Conversion/TorchToTosa/TosaLegalizeCommon.cpp

should fix it.

[eric-k256]

Yes, that's the basic idea. I've got a commit I'm about to add that fixes it locally. Some of the checks in the lit tests needed updating also.

[stellaraccident]

Yes, that's the basic idea. I've got a commit I'm about to add that fixes it locally. Some of the checks in the lit tests needed updating also.

Cool, thanks. You just convinced me to go eat breakfast vs coming up with a fix :)

[stellaraccident]

With that patch, feel free to squash and merge if it turns green and I'm not back yet.

[stellaraccident]

Yes, that's the basic idea. I've got a commit I'm about to add that fixes it locally. Some of the checks in the lit tests needed updating also.

Looks like it might need some more work. Crash on that test now...

[eric-k256]

Yeah, not sure what's going on yet as it passed locally. Will see if I can reproduce the crash.

[ramiro050]

I think the data being passed to

torch-mlir/lib/Conversion/TorchToTosa/TorchToTosa.cpp

Line 723 in 82456ee

DenseIntElementsAttr::get(reduceDimsType, llvm::ArrayRef(reduceDims));

needs to match the type. In other words, the reduceDims probably needs to be a vector of int32_t values.

[eric-k256]

Thanks @ramiro050 I think you're right, and I can reproduce locally now, so should be able to verify before pushing the fix.

[stellaraccident]

Looks in better shape now. I found an accidental dump that got left in. Pushing a patch to remove that.

[eric-k256]

Things are looking good in CI now. Thanks for your patience.

[stellaraccident]

Days since the mlir-hlo dep screwed us: 0

One line fix on Windows. No way to actually make it because the Google repos are impenetrable.