I have a patch I'd like to commit that adds commentary to asm files about which TableGen pattern generated a particular instruction. The output looks like this: cvtpd2ps %xmm0, %xmm0 # source.c:39 # Src: (intrinsic_wo_chain:v4f32 927:iPTR, VR128:v2f64:$src) # Dst: (Int_CVTPD2PSrr:v4f32 VR128:v2f64:$src) This is enormously helpful when trying to track down codegen bugs but clutters the asm file pretty badly for "ordinary" users. Right now I have this under control of a separate -asm-pattern flag. All other asm commentary is controlled by -asm-verbose. Is a new flag appropriate for this or should I just put it under -asm-verbose with everything else? If we want a new flag, does anyone have a spelling they'd recommend? I'm not particularly fond of -asm-pattern but it was the best concise name my uncreative brain could conjur up. -Dave
On Jul 11, 2011, at 2:48 PM, David Greene wrote:> I have a patch I'd like to commit that adds commentary to asm files > about which TableGen pattern generated a particular instruction. The > output looks like this: > > cvtpd2ps %xmm0, %xmm0 # source.c:39 > # Src: (intrinsic_wo_chain:v4f32 927:iPTR, VR128:v2f64:$src) > # Dst: (Int_CVTPD2PSrr:v4f32 VR128:v2f64:$src) > > This is enormously helpful when trying to track down codegen bugs but > clutters the asm file pretty badly for "ordinary" users. > > Right now I have this under control of a separate -asm-pattern flag. > All other asm commentary is controlled by -asm-verbose. > > Is a new flag appropriate for this or should I just put it under > -asm-verbose with everything else? If we want a new flag, does anyone > have a spelling they'd recommend? I'm not particularly fond of > -asm-pattern but it was the best concise name my uncreative brain could > conjur up.I wouldn't mind this sort of thing, but yes a separate option is definitely needed :) -eric
On Jul 11, 2011, at 2:48 PM, David Greene wrote:> I have a patch I'd like to commit that adds commentary to asm files > about which TableGen pattern generated a particular instruction. The > output looks like this: > > cvtpd2ps %xmm0, %xmm0 # source.c:39 > # Src: (intrinsic_wo_chain:v4f32 927:iPTR, VR128:v2f64:$src) > # Dst: (Int_CVTPD2PSrr:v4f32 VR128:v2f64:$src) > > This is enormously helpful when trying to track down codegen bugs but > clutters the asm file pretty badly for "ordinary" users. > > Right now I have this under control of a separate -asm-pattern flag. > All other asm commentary is controlled by -asm-verbose. > > Is a new flag appropriate for this or should I just put it under > -asm-verbose with everything else? If we want a new flag, does anyone > have a spelling they'd recommend? I'm not particularly fond of > -asm-pattern but it was the best concise name my uncreative brain could > conjur up.How do you plan to implement this? This is a compiler-hacker-only feature, can it be protected fully in !NDEBUG code? -Chris
Chris Lattner <clattner at apple.com> writes:>> Is a new flag appropriate for this or should I just put it under >> -asm-verbose with everything else? If we want a new flag, does anyone >> have a spelling they'd recommend? I'm not particularly fond of >> -asm-pattern but it was the best concise name my uncreative brain could >> conjur up. > > How do you plan to implement this? This is a compiler-hacker-only > feature, can it be protected fully in !NDEBUG code?The codegen portion certainly can be protected with !NDEBUG. The change has two parts: a TableGen enhancement to output a table of pattern strings and some code to pair up MachineInstrs with the appropriate index into the pattern string table and an AsmPrinter portion to actually print the comments. The latter can be easily controlled via NDEBUG. The former is more difficult since at the time TableGen is run we don't know how the rest of the compiler will be built. How would you prefer this work? Even if the output were controlled by NDEBUG, I feel the added pattern comments make the asm file too cluttered even for day-to-day compiler developers. This is really a feature to debug instruction selection problems. That's why I put it under the control of a separate option. -Dave