mstorsjo added a comment.

In D128268#3614661 <https://reviews.llvm.org/D128268#3614661>, @labath wrote:

> In D128268#3611053 <https://reviews.llvm.org/D128268#3611053>, @mstorsjo 
> wrote:
>
>> The odd thing about the second one is the added hardcoded 
>> `AddArch(ArchSpec("i686-pc-windows"));` and 
>> `AddArch(ArchSpec("i386-pc-windows"));` at the top of `PlatformWindows.cpp`.
>>
>> Anyway, it does seem to work fine in my quick test to get rid of this 
>> duality, so I'll post a patch that does that.
>
> I think those hardcoded lines are there precisely because of the duality 
> (i.e. wanting to support both i386 and i686 regardless of what the host layer 
> reports).

Both that, and because the i386+i686 fat binary arches from ObjectFile 
triggered the more elaborate arch validation, these had to be present to make 
it work. Anyway, after getting rid of the duality, those hardcoded ones don't 
seem to be needed either.

> If that is removed, maybe all of that can be changed to  the pattern used in 
> other platform plugins 
> <https://github.com/llvm/llvm-project/blob/main/lldb/source/Plugins/Platform/Linux/PlatformLinux.cpp#L114>.

That looks reasonable!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128268

_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to