Hi Andrew, > Sorry it's taken soooo long to review this patch. > This is understandable, it is a quite large patch and changes a lot of items.
> In general I like this, and think it's a step in the right direction. > I have a few pretty minor concerns, but hopefully nothing too > contentious. > > In general I like the use of the *.def files, but my only concern is > the lack of explanation in any of them about what they're for. It > would be nice if each had a summary of what each field represents to > make it easier to add new entries. This is a good point, I will add extra info on how to add new entries. > > The use of arc_seen_options can, I think be replaced by using > global_opts_seen.x_target_flags, or maybe there's a detail I'm not > understanding, in which case maybe a comment somewhere to explain why > those two things are different. I cannot remember why I haven't use the global_options_set, but I will try as u suggested. > > The only thing I dislike in this patch is the switch from 'arc_cpu' to > a set of global boolean flags. I can't see why we'd ever want more > than one of those architecture flags to be true, so I'd rather we > switched back to an enum if that's possible. > I remember, initially I wanted to use enums, but I bumped into an issue, don't remember exactly what. As far as I can see now, it perfectly make sense your comment. I will come back to u with extra info/revised patch asap, Claudiu