[PATCH] D71219: Fix conflict value for metadata "Objective-C Garbage Collection" in the mix of swift and Objective-C bitcode

[Jin Lin via Phabricator via llvm-commits]

jinlin added a comment.

In D71219#1891857 <https://reviews.llvm.org/D71219#1891857>, @steven_wu wrote:

> In D71219#1890444 <https://reviews.llvm.org/D71219#1890444>, @jinlin wrote:
>
> > In D71219#1791936 <https://reviews.llvm.org/D71219#1791936>, @steven_wu wrote:
> >
> > > > 
> > > > 
> > > > 1. I will add a  llvm-link tests for combining objc and swift bitcode.
> > > > 2. What do you mean by "codegen tests for correct value in the object file"? There is no "object file" in the picture. Are you asking for a test that checks the value in the output of llvm-link?
> > >
> > > I think the decision was to break apart the current "Objective-C Garbage Collection" metadata. You need codegen test to test that OBJC_IMAGE_INFO is generated correctly after breaking apart the value.
> >
> >
> > I have question regarding this. Let's assume we have one assembly file from Objective-C and another one from Swift.
> >
> > Objective-C assembly:
> > L_OBJC_IMAGE_INFO:
> >
> >   	.long	0
> >   	.long	64
> >
> >
> > Swift assembly:
> >
> > L_OBJC_IMAGE_INFO:
> >
> >   	.long	0
> >   	.long	83953472
> >
> >
> > Let's assume that the llvm-link can link Swift bitcode and Objective-C bit successfully and the llc can generate the assembly for the merged output bitcode.
> >
> > In the final assembly file, what value do you expect for L_OBJC_IMAGE_INFO?
>
>
> You can easily test that out with ld64. It depends on the bits in the current garbage collection fields. Most of the fields are useless now (only there for GC which is obsoleted), that is why the module flag uses `Override`. Some bits in there are taking `Max`, like `OBJC_IMAGE_HAS_CATEGORY_CLASS_PROPERTIES`. Swift version should be taking `Error` since mismatch is a failure, unless it is not set.




> If you are breaking the module flag "Objective-C Garbage Collection" into all the corresponding bits, you can correctly specify all the right behavior for the bitfield.
> 
>> 
>> 
>>> 
>>> 
>>>> 3. Let us keep the auto upgrade to a different future diff to keep each diff self contained. Is that ok?
>>> 
>>> I disagree with that. That is leaving a gap for incompatible bitcode.
>>> 
>>> Steven



In D71219#1892512 <https://reviews.llvm.org/D71219#1892512>, @steven_wu wrote:

> > I agree with all of this except that I'm not sure there's any situation where ObjC and Swift translation units should be disagreeing about the ObjC-specific parts of the OBJC_IMAGE_INFO.
>
> The lower bits of the flag is for objc info and here is the definition:
>  #define OBJC_IMAGE_SUPPORTS_GC						(1<<1)
>  #define OBJC_IMAGE_REQUIRES_GC						(1<<2)
>  #define OBJC_IMAGE_OPTIMIZED_BY_DYLD				(1<<3)
>  #define OBJC_IMAGE_SUPPORTS_COMPACTION				(1<<4)
>  #define OBJC_IMAGE_IS_SIMULATED						(1<<5)
>  #define OBJC_IMAGE_HAS_CATEGORY_CLASS_PROPERTIES	(1<<6)
>
> The 0x40 bit is actually OBJC_IMAGE_HAS_CATEGORY_CLASS_PROPERTIES, which probably can differ from translation unit to translation unit even within ObjC. Some of the other bits should not be different. That is why I say you can break the bits down even more if needed.


Do you suggest I should break apart the current "Objective-C Garbage Collection" metadata for both Swift and Objective-C?


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

https://reviews.llvm.org/D71219