On Feb 15, 2008, at 11:17 AM, Devang Patel wrote:> > On Feb 15, 2008, at 10:27 AM, Chris Lattner wrote: > >> >> On Feb 15, 2008, at 10:23 AM, Devang Patel wrote: >> >>> >>> On Feb 15, 2008, at 10:08 AM, Chris Lattner wrote: >>> >>>>>> Alternatively I can take the Padding bit into account in the >>>>>> StructType::get code somehow. Anyone have a strong opinion? >>>>> >>>>> Shouldn't it be a map from the gcc type to the padding info? >>>>> That said, you can get rid of the padding info as far as I'm >>>>> concerned. However Chris might have a different opinion - I >>>>> think he introduced it. >>>> >>>> I don't think I introduced it (was it Devang?). >>> >>> Yup. PR 1278 >> >> Ok! Can you please fix it to index by GCC type? There is a many to >> one mapping between gcc types and llvm types. > > This is tricky and probably won't work. Padding info is in llvm struct > type and CopyAggregate() is operating on llvm type. There is not any > way to map llvm type back to gcc type. Am I missing something ?I don't think so, I have reached the same conclusion. You can pass the gcc type into CopyAggregate, but it's recursive, and there's no way to get the gcc type for the fields. You would have to walk the gcc type in parallel with the llvm type, which at best involves duplicating a lot of code and is quite error prone.
On Feb 15, 2008, at 11:22 AM, Dale Johannesen wrote:> > On Feb 15, 2008, at 11:17 AM, Devang Patel wrote: > >> >> On Feb 15, 2008, at 10:27 AM, Chris Lattner wrote: >> >>> >>> On Feb 15, 2008, at 10:23 AM, Devang Patel wrote: >>> >>>> >>>> On Feb 15, 2008, at 10:08 AM, Chris Lattner wrote: >>>> >>>>>>> Alternatively I can take the Padding bit into account in the >>>>>>> StructType::get code somehow. Anyone have a strong opinion? >>>>>> >>>>>> Shouldn't it be a map from the gcc type to the padding info? >>>>>> That said, you can get rid of the padding info as far as I'm >>>>>> concerned. However Chris might have a different opinion - I >>>>>> think he introduced it. >>>>> >>>>> I don't think I introduced it (was it Devang?). >>>> >>>> Yup. PR 1278 >>> >>> Ok! Can you please fix it to index by GCC type? There is a many to >>> one mapping between gcc types and llvm types. >> >> This is tricky and probably won't work. Padding info is in llvm >> struct >> type and CopyAggregate() is operating on llvm type. There is not any >> way to map llvm type back to gcc type. Am I missing something ? > > > I don't think so, I have reached the same conclusion. You can pass > the gcc type into CopyAggregate, but it's recursive, and there's no > way to get the gcc type for the fields. You would have to walk the > gcc type in parallel with the llvm type, which at best involves > duplicating a lot of code and is quite error prone....but giving up in this case is easy enough, ok, I can do that.
On Feb 15, 2008, at 11:27 AM, Dale Johannesen wrote:>> I don't think so, I have reached the same conclusion. You can pass >> the gcc type into CopyAggregate, but it's recursive, and there's no >> way to get the gcc type for the fields. You would have to walk the >> gcc type in parallel with the llvm type, which at best involves >> duplicating a lot of code and is quite error prone. > > ...but giving up in this case is easy enough, ok, I can do that.Cool, thanks Dale! I think it would be reasonable to give up in nested struct cases, etc. We can always improve it later, and that will get us the obvious case in PR1278. -Chris