[PATCH] D64931: Change X86 datalayout for three address spaces that specify pointer sizes.

dyung added a comment. In D64931#1647880 , @dyung wrote: > Hi, this is causing 3 test failures on the PS4 linux bot. The changes may not > have been initially flagged because a different issue was causing a build > failure which masked the problem. I hav

[PATCH] D64931: Change X86 datalayout for three address spaces that specify pointer sizes.

dyung added a comment. Hi, this is causing 3 test failures on the PS4 linux bot. The changes may not have been initially flagged because a different issue was causing a build failure which masked the problem. I have bisected the test failures to this change though. http://lab.llvm.org:8011/bui

[PATCH] D64931: Change X86 datalayout for three address spaces that specify pointer sizes.

akhuang closed this revision. akhuang added a comment. Commited in r370083 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64931/new/ https://reviews.llvm.org/D64931 ___ cfe-commits mailing list cfe-comm

[PATCH] D64931: Change X86 datalayout for three address spaces that specify pointer sizes.

akhuang added a comment. Pinging reviewers -- are there any other concerns on this patch? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64931/new/ https://reviews.llvm.org/D64931 ___ cfe-commits mailin

[PATCH] D64931: Change X86 datalayout for three address spaces that specify pointer sizes.

rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. I think we're ready to proceed here, lgtm. Shout if I've misrepresented anything. :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64931/new/ https:/

[PATCH] D64931: Change X86 datalayout for three address spaces that specify pointer sizes.

arsenm added a comment. In D64931#1633669 , @akhuang wrote: > > Address space have backend defined semantics, and aren’t really reserved > > for front end use. I think the fact that non-0 address spaces on X86 > > codegen the same as address space 0 and

[PATCH] D64931: Change X86 datalayout for three address spaces that specify pointer sizes.

akhuang added a comment. > Address space have backend defined semantics, and aren’t really reserved for > front end use. I think the fact that non-0 address spaces on X86 codegen the > same as address space 0 and could be used for something by a front end is an > accident of how SelectionDAG is

[PATCH] D64931: Change X86 datalayout for three address spaces that specify pointer sizes.

arsenm added a comment. In D64931#1622039 , @lebedev.ri wrote: > In D64931#1622038 , @akhuang wrote: > > > @lebedev.ri The test case datalayout strings were changed because somewhere > > llvm asserts that the strin

[PATCH] D64931: Change X86 datalayout for three address spaces that specify pointer sizes.

lebedev.ri added a comment. In D64931#1622144 , @akhuang wrote: > > Can you post a reproducer? > > Turns out I just didn't have assertions enabled. With assertions the changed > test cases should fail. > > > I think this is precisely what was discussed in

[PATCH] D64931: Change X86 datalayout for three address spaces that specify pointer sizes.

akhuang added a comment. > Can you post a reproducer? Turns out I just didn't have assertions enabled. With assertions the changed test cases should fail. > I think this is precisely what was discussed in replies to RFC - this > hardcodes these address spaces, and thus either makes them unaval

[PATCH] D64931: Change X86 datalayout for three address spaces that specify pointer sizes.

lebedev.ri added a comment. In D64931#1622038 , @akhuang wrote: > @lebedev.ri The test case datalayout strings were changed because somewhere > llvm asserts that the string in the IR matches the backend datalayout. I > don't know why I wasn't getting the

[PATCH] D64931: Change X86 datalayout for three address spaces that specify pointer sizes.

akhuang added a comment. @lebedev.ri The test case datalayout strings were changed because somewhere llvm asserts that the string in the IR matches the backend datalayout. I don't know why I wasn't getting the assert error now, but I think they'll all have to be changed if we change the X86 dat

[PATCH] D64931: Change X86 datalayout for three address spaces that specify pointer sizes.

lebedev.ri added a reviewer: reames. lebedev.ri added inline comments. Comment at: llvm/lib/Target/X86/X86TargetMachine.cpp:119-120 + // Address spaces for 32 bit signed, 32 bit unsigned, and 64 bit pointers. + Ret += "-p253:32:32-p254:32:32-p255:64:64"; + I

[PATCH] D64931: Change X86 datalayout for three address spaces that specify pointer sizes.

akhuang updated this revision to Diff 214202. akhuang added a comment. Remove test case changes. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64931/new/ https://reviews.llvm.org/D64931 Files: clang/lib/Basic/Targets/OSTargets.h clang/lib/Basi

[PATCH] D64931: Change X86 datalayout for three address spaces that specify pointer sizes.

akhuang added a comment. For some reason the tests were failing before without the datalayout change? I'm not sure why, but I changed them back and they're fine. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64931/new/ https://reviews.llvm.org/D64

[PATCH] D64931: Change X86 datalayout for three address spaces that specify pointer sizes.

rnk added inline comments. Comment at: clang/lib/Basic/Targets/OSTargets.h:771-772 } else if (Triple.getArch() == llvm::Triple::x86) { - this->resetDataLayout("e-m:e-p:32:32-i64:64-n8:16:32-S128"); + this->resetDataLayout("e-m:e-p:32:32-p253:32:32-p254:32:32-p255:6

[PATCH] D64931: Change X86 datalayout for three address spaces that specify pointer sizes.

lebedev.ri added inline comments. Comment at: clang/lib/Basic/Targets/OSTargets.h:771-772 } else if (Triple.getArch() == llvm::Triple::x86) { - this->resetDataLayout("e-m:e-p:32:32-i64:64-n8:16:32-S128"); + this->resetDataLayout("e-m:e-p:32:32-p253:32:32-p254:32:32

[PATCH] D64931: Change X86 datalayout for three address spaces that specify pointer sizes.

lebedev.ri added a comment. Why do you need to change (update) the datalayout in every single test? That looks extremely noisy and hides the actually-needed changes. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64931/new/ https://reviews.llvm.org/

[PATCH] D64931: Change X86 datalayout for three address spaces that specify pointer sizes.

akhuang added a comment. The llvm-dev discussion is here http://lists.llvm.org/pipermail/llvm-dev/2019-July/134035.html I think the consensus is that it should be fine to change the data layout. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64931/n

[PATCH] D64931: Change X86 datalayout for three address spaces that specify pointer sizes.

rnk added a comment. I suggested to @akhuang offline that we should discuss this on llvm-dev. I'll add some other X86 reviewers. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64931/new/ https://reviews.llvm.org/D64931 __