[PATCH] Cleanup inconsistent MachineVerifier+Print usage

Matthias Braun matze at braunis.de
Mon Dec 1 14:07:10 PST 2014


Hi Quentin,

thanks for the review!

> On Dec 1, 2014, at 1:55 PM, Quentin Colombet <qcolombet at apple.com> wrote:
> 
> Hi Matthias,
> 
> I like the direction of this!
> 
> Here are a few commets on 0002-CodeGen-Add-print-and-verify-pass-after-each-Machine.patch:
>   if (getARMSubtarget().isThumb2())
>     addPass(createThumb2ITBlockPass());
> -
> -  return true;
> 
> Shouldn’t we have a call to the verifier in the else case?
> 
> 
> -      addPass(createHexagonCFGOptimizer(TM));
> -  return false;
> +      addPass(createHexagonCFGOptimizer(TM), false);
> 
> Although I like the verbosity of this new approach, shouldn’t this call be a …, false, false?
> 
> There are a couple of other places like this. I think we should decide whether or not we want the whole verbosity. I leave that to the related target owners.
> Do not hesitate to ping them!
I assumed having a few more intermediate steps printed when -print-machine-instrs is enabled is a good thing. Anyway CC'ing the target owners to hear some more opinions on whether it is a problem/unwanted to print machine code between every pass instead of just where it is manually requested with printAndVerify().

Greetings
	Matthias

> 
> Thanks,
> -Quentin
> 
> On Dec 1, 2014, at 11:12 AM, Matthias Braun <matze at braunis.de> wrote:
> 
>> Hi Evan, list,
>> 
>> attached are patches that cleanup some of the mess in CodeGen/Passes.cpp; specifically they change the behaviour to add print+verify passes after each MachineFunction pass by default instead of on request. This should help debugging as you don't miss intermediate steps that are not printed and should improve dicipline to produce correct machine code as newly written passes will have verification enabled by default.
>> 
>> Greetings
>>   Matthias
>> 
>> <0001-CodeGen-Let-MachineVerifierPass-own-its-banner-strin.patch><0002-CodeGen-Add-print-and-verify-pass-after-each-Machine.patch><0003-X86-do-not-disable-machine-verification-anymore.patch><0004-ARM-do-not-disable-machine-verification-anymore.patch><0005-AARCH64-do-not-disable-machine-verification-anymore.patch>_______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
> 
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits





More information about the llvm-commits mailing list