Page MenuHomePhabricator

[X86] Handle localdynamic TLS model in x32 mode

Authored by hvdijk on Dec 6 2020, 12:51 PM.



D92346 added TLS_(base_)addrX32 to handle TLS in x32 mode, but missed the different TLS models. This diff fixes the logic for the local dynamic model where RAX was used when EAX should be, and extends the tests to cover all four TLS models.


Diff Detail

Event Timeline

hvdijk created this revision.Dec 6 2020, 12:51 PM
hvdijk requested review of this revision.Dec 6 2020, 12:51 PM
pengfei added inline comments.Dec 6 2020, 7:16 PM

What're this and test11 testing for? It seems you only changed for LocalDynamic model in this patch.

hvdijk added inline comments.Dec 7 2020, 1:23 AM

That is correct, I did not update anything for the InitialExec and LocalExec TLS models. I added these tests not because this diff fixes bugs in them, but because like LocalDynamic, these models were also not covered by existing tests. Despite the title (I wrote the title and description afterwards), the goal of this diff was not specifically to make sure that the LocalDynamic TLS model is working, but that all TLS models are working. For this, I first made sure that all TLS models are tested, and then fixed the code generation where needed, which turned out to only be that of the LocalDynamic model.

pengfei added inline comments.Dec 7 2020, 4:34 AM

Thanks Harald, I just saw you had mentioned it in the summary. Sorry for the noise.

RKSimon added inline comments.Dec 7 2020, 10:26 AM

(style) Fix the case of the variables: Is64Bit Is64BitLP64

hvdijk updated this revision to Diff 309956.Dec 7 2020, 10:46 AM

Renamed is* parameters to Is*.

hvdijk marked an inline comment as done.Dec 7 2020, 10:46 AM
RKSimon accepted this revision.Dec 8 2020, 4:04 AM

LGTM - cheers

This revision is now accepted and ready to land.Dec 8 2020, 4:04 AM
This revision was landed with ongoing or failed builds.Dec 8 2020, 1:06 PM
This revision was automatically updated to reflect the committed changes.