Page MenuHomePhabricator

[ProcessWindows] Choose a register context file by prepocessor
ClosedPublic

Authored by tatyana-krasnukha on Jul 29 2019, 10:44 AM.

Diff Detail

Repository
rL LLVM

Event Timeline

xbolva00 added inline comments.
Common/CMakeLists.txt
25 ↗(On Diff #212192)

Maybe add FIXME or TODO to revisit state of https://gitlab.kitware.com/cmake/cmake/issues/15170 ?

Common/CMakeLists.txt
25 ↗(On Diff #212192)

I'm not sure we should rollback to that approach also due to the comment

# TODO build these unconditionally as we cannot do cross-debugging or WoW

compnerd added inline comments.Jul 29 2019, 2:04 PM
Common/CMakeLists.txt
28 ↗(On Diff #212192)

At this point, I would say its better to just merge it into the main source list.

Common/x64/RegisterContextWindows_x64.h
47 ↗(On Diff #212192)

Can you push this inside the include guards please?

Common/x86/RegisterContextWindows_x86.h
51 ↗(On Diff #212192)

Can you sink the check here inside the include guards please?

The change looks fine to me. I also agree with @compnerd's comments.

Addressed comments

I wonder if directories x86 and x64 are needed. Should I remove them to make hierarchy consistent with D63165?

labath accepted this revision.Jul 30 2019, 10:44 AM

I wonder if directories x86 and x64 are needed. Should I remove them to make hierarchy consistent with D63165?

I don't have any opinion on that.

This revision is now accepted and ready to land.Jul 30 2019, 10:44 AM
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptJul 31 2019, 5:00 AM