[PATCH] D52156: [LLD] [COFF] Alternative ARM range thunk algorithm

[Rui Ueyama via Phabricator via llvm-commits]

ruiu added inline comments.


================
Comment at: COFF/Writer.cpp:385
+static std::pair<Defined *, bool>
+getThunk(DenseMap<uint64_t, Defined *> &LastThunks, Defined *Target, uint64_t P,
+         uint16_t Type, int Margin) {
----------------
mstorsjo wrote:
> ruiu wrote:
> > The relationship between section and its thunk is 1:N. I believe that's why you are using a map to manage thunk -> symbol mapping.
> > 
> > However, I think you visit sections in the increasing address order, so if the last thunk is not reachable, no other thunks are reachable. Can you use the fact?
> > The relationship between section and its thunk is 1:N. I believe that's why you are using a map to manage thunk -> symbol mapping.
> 
> For every target Symbol, there can be N thunks that point to it, yes.
> 
> > However, I think you visit sections in the increasing address order, so if the last thunk is not reachable, no other thunks are reachable. Can you use the fact?
> 
> Yes - I'm using exactly this fact. This map allows me to look up the last thunk pointing to each specific RVA. (In the previous version, I had something similar to `DenseMap<uint64_t, vector<Defined *>>`, to allow finding all thunks for a specific target. But as you say, as we progress linearly we only need to keep track of the last thunk for each target.)
I wonder if you can eliminate map lookups (which is slow). Since the relocation is 1:N, you can add a `Defined *` to a symbol to represent the relationship, no?


https://reviews.llvm.org/D52156