Page MenuHomePhabricator

Delete le32/le64 targets
AcceptedPublic

Authored by MaskRay on Wed, Apr 21, 11:03 AM.

Details

Summary

They are unused now.

Note: NaCl is still used and is currently expected to be needed until 2022-06
(https://blog.chromium.org/2020/08/changes-to-chrome-app-support-timeline.html).

Diff Detail

Event Timeline

MaskRay created this revision.Wed, Apr 21, 11:03 AM
MaskRay requested review of this revision.Wed, Apr 21, 11:03 AM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptWed, Apr 21, 11:03 AM
dschuff accepted this revision.Wed, Apr 21, 5:15 PM

Thanks. I had heard in the past that there were some other folks who had used le32/le64 as a "generic" target (in fact that's why it's named so generically, rather than being called "pnacl" or similar) but I haven't heard of anything recently, and as you can see nobody has upstreamed any support for other OS or target specializations or asked to collaborate on it. Practically speaking even a target that wants fairly generic bitcode would probably want its own triple, so unless this removal captures someone's attention who wants to keep maintaining it, this should be fine to remove.

This revision is now accepted and ready to land.Wed, Apr 21, 5:15 PM
This revision was landed with ongoing or failed builds.Wed, Apr 21, 6:44 PM
This revision was automatically updated to reflect the committed changes.
abadams added a subscriber: abadams.EditedThu, Apr 22, 9:01 AM

Thanks. I had heard in the past that there were some other folks who had used le32/le64 as a "generic" target (in fact that's why it's named so generically, rather than being called "pnacl" or similar) but I haven't heard of anything recently, and as you can see nobody has upstreamed any support for other OS or target specializations or asked to collaborate on it. Practically speaking even a target that wants fairly generic bitcode would probably want its own triple, so unless this removal captures someone's attention who wants to keep maintaining it, this should be fine to remove.

I think that was us, the Halide language. Any suggested alternative for us to use now? This does indeed break all our llvm usage.

EDIT: We are testing to see if using arm triples with an unknown OS works: https://github.com/halide/Halide/pull/5934

The code this is applied to is very careful to not do anything that depends on the triple, which is why we were using the generic one. Given that, it's possible that any old triple will work for us.

srj added a subscriber: srj.Thu, Apr 22, 9:23 AM
srj added a comment.Thu, Apr 22, 9:28 AM

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.

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.

srj added a comment.Thu, Apr 22, 10:05 AM

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.

Yeah, the first simple attempt in https://github.com/halide/Halide/pull/5934 doesn't work (many things are broken). Temporarily reverting this would be a huge huge favor to us.

MaskRay reopened this revision.Thu, Apr 22, 10:19 AM
This revision is now accepted and ready to land.Thu, Apr 22, 10:19 AM

Would it make sense for you to to upstream an LLVM target such as le32-halide? (Or perhaps even arm32-halide or some other?) Then you'd actually have more control over your own codegen, datalayout, etc.

maskray: your undelete didn't add back BuiltinsLe64.def so builds are broken atm

maskray: your undelete didn't add back BuiltinsLe64.def so builds are broken atm

Fixed by vitalybuka