Tim Armstrong has posted comments on this change.

Change subject: Fix LLVM linker bug
......................................................................


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/2656/2/source/llvm/llvm-3.8.0-patches/0001-llvm-link-struct-types.patch
File source/llvm/llvm-3.8.0-patches/0001-llvm-link-struct-types.patch:

Line 53: if (Ty->isOpaque())
> Why the distinction between opaque vs non-opaque type at this function befo
The previous code tried to have it so that each struct type was only stored in 
one hash table, I guess to avoid duplication. That doesn't work properly though 
because hasType() needs to be able to look up opaque and non-opaque types by 
identity.

The only advantage of checking Ty->isOpaque() here I can see here is that it 
saves a hash table lookup.


-- 
To view, visit http://gerrit.cloudera.org:8080/2656
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I8aef7f79c8d905b267f5acc3392384b1b333e7d3
Gerrit-PatchSet: 2
Gerrit-Project: Toolchain
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong <[email protected]>
Gerrit-Reviewer: Casey Ching <[email protected]>
Gerrit-Reviewer: Michael Ho
Gerrit-Reviewer: Michael Ho <[email protected]>
Gerrit-Reviewer: Tim Armstrong <[email protected]>
Gerrit-HasComments: Yes

Reply via email to