This is an archive of the discontinued LLVM Phabricator instance.

[Haiku] Support __float128 for Haiku x86 and x86_64
AbandonedPublic

Authored by kristina on Oct 25 2018, 6:00 AM.

Details

Summary

This patch addresses a compilation error with clang when running in Haiku being unable to compile code using float128 (throws compilation error such as 'float128 is not supported on this target').

Diff Detail

Repository
rC Clang

Event Timeline

return created this revision.Oct 25 2018, 6:00 AM

Thanks smeenai! Any chance of this one getting into 7.x.x? It's technically a bug fix... I think it's the last major hurdle to Haiku compiling (easily) with clang vs gcc.

kristina added a subscriber: kristina.

Do you mind resubmitting those with context? Also I would suggest asking Tom Stellard as he's in charge of handling cherrypicking patches to go into releases once the major rolls over and I think we're pretty close (?) to 7.0.1.

js added a subscriber: js.Nov 21 2018, 5:29 PM
js added inline comments.
lib/Basic/Targets/OSTargets.h
262

This seems weird. Shouldn't by stdlibc++ and not by the compiler?

js requested changes to this revision.Nov 21 2018, 5:30 PM
js added inline comments.
lib/Basic/Targets/OSTargets.h
262

+be defined

This revision now requires changes to proceed.Nov 21 2018, 5:30 PM

Linux, Solaris and OpenBSD also define it where appropriate, it depends on the target. This is why it's much easier to review when full context (diff with -U 99999) is provided, and means that it has a much lower change of patch doing something unexpected.

Indeed. I agree the _GLIBCXX_USE_FLOAT128 is misplaced there. That comes from config.h in libstdc++. That's working around an issue in Haiku's build (libstdc++ is built by gcc, but then we try and use it with clang later)
I can remove that.

Is the FLOAT128 desireable?

kristina requested changes to this revision.Nov 22 2018, 3:29 PM

Indeed. I agree the _GLIBCXX_USE_FLOAT128 is misplaced there. That comes from config.h in libstdc++. That's working around an issue in Haiku's build (libstdc++ is built by gcc, but then we try and use it with clang later)
I can remove that.

Is the FLOAT128 desireable?

Sorry I was referring to __FLOAT128__ being standard, the other define shouldn't be there IMO. But then again I'm not an expert on Haiku, __FLOAT128__ seems fine to me, the other change looks more questionable, if anyone is familiar with Haiku internals, please chime in.

Ping. Is the author still around? I'm happy to take over this revision if not, and add __FLOAT128__ but unless someone says _GLIBCXX_USE_FLOAT128 is actually defined by the toolchain, I'll remove that line, it looks like it really shouldn't be defined in the toolchain, but I don't have a Haiku toolchain around so I can't say for sure.

sorry, busy weekend. If you find the time feel free :-)

  • _GLIBCXX_USE_FLOAT128 can (and should) go
  • everything else is valid per the discussions here.

Otherwise i'll pick up in the next few days and add the requested context as well.

I don't have permission to update this request, so I created https://reviews.llvm.org/D54901 with the updates recommended.

kristina commandeered this revision.Nov 28 2018, 4:43 AM
kristina edited reviewers, added: return; removed: kristina.

New revision in D54901.