[llvm-commits] PATCH: Refactor LLVM's triple normalization

Duncan Sands baldrick at free.fr
Wed Feb 22 07:34:46 PST 2012

Hi Chandler,

> Latest patch in the series re-working llvm::Triple to be more suitable for use
> inside of a cross-compiling FE.

can't you easily get the desired effect with the existing logic by changing the
Components array from an array of StringRefs to an array of
(StringRef, unsigned) pairs, where the second component tells you what the
original position was.  I.e. when you initialize Components, you initialize
the i'th element to (Whatever_String_Ref, i).  At places where you insert
an empty component (i.e. something which didn't have an original position)
you can insert a ("", ~0U) pair rather than "".  Then at the end you
automatically get the permutation as well as the permuted strings.

> I think this version is easier to read. It burns most of the lines of code on
> asserts and an enum now. It is however much less efficient. ;]

Indeed.  I have to ask: is there really any point to this rewrite?  I think
your motivation was to make it easier to extract the permutation, but as far
as I can see that's trivial (see above).  So that leaves the goodness of
producing nicer, more understandable and more maintainable code.  I agree that
the arch/vendor/etc parsing parts of the existing code could probably be
cleaner and factorized out.  I mean these bits:

   ArchType Arch = UnknownArch;
   if (Components.size() > 0)
     Arch = ParseArch(Components[0]);
   if (Components.size() > 3)
     Environment = ParseEnvironment(Components[3]);


       case 0:
         Arch = ParseArch(Comp);
         Valid = Arch != UnknownArch;
       case 3:
         Environment = ParseEnvironment(Comp);
         Valid = Environment != UnknownEnvironment;

Ignoring these bits, what's left is pretty short and clean.  I'm not sure that
what you are proposing is any better (and it loses a feature: being able to
handle strings which parse as more than one thing, which was useful in the past
and maybe in the future).

Ciao, Duncan.

More information about the llvm-commits mailing list