[PATCH] D118072: [X86] [CodeView] Add codeview mappings for registers ST0-ST7

Mon Jan 24 15:25:13 PST 2022

aganea accepted this revision.
aganea added a comment.

In D118072#3267627 <https://reviews.llvm.org/D118072#3267627>, @mstorsjo wrote:

> In D118072#3267462 <https://reviews.llvm.org/D118072#3267462>, @aganea wrote:
>
>> The feature can still be enabled with `-mllvm -experimental-debug-variable-locations=true`, so D116821 <https://reviews.llvm.org/D116821> wasn't completely reverted.
>>  (l
>> Why not adding your initial bug repro as a test, along with the flags above on the cmd line? It also looks like that prior mappings were added without test coverage.
>
> Ok, I made such a testcase. I'm a bit afraid that it might be a little brittle in the long run though...

I was simply surprised that a such simple testcase wasn't catched elsewhere. Could you please explain 'brittle'? Do you think the outcome of this test isn't stable in the long run?
In any case LGTM.
I'll let the choice (to add a test or not) to your discretion ;-) I felt that it could make you loose time again later down the line...

In D118072#3267599 <https://reviews.llvm.org/D118072#3267599>, @rnk wrote:

> I think the plan is to reland the feature, so we'll have test coverage for this soon enough.

I was under the impression that the discussions in D11681 <https://reviews.llvm.org/D11681> were revolving around resources not correctness?

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D118072/new/

https://reviews.llvm.org/D118072