[llvm] r333091 - Remove DEBUG macro.

Nicola Zaghen via llvm-commits llvm-commits at lists.llvm.org
Fri May 25 03:45:32 PDT 2018


Hi Daniel,

The removal was discussed on the mailing list back in March 
http://lists.llvm.org/pipermail/llvm-dev/2018-March/122097.html, but you 
make a good point: an email to let people know that the change is 
actually in and with some instructions would be a good thing to do. I'll 
look into it now.

Regarding the quick replace -> remove, this was also part of the email 
(and also the review of the change). It was done so quickly because 
every day people would add new uses of the DEBUG() macro as it would 
have kept working the same way, which would then left us with two 
equivalent macros for a long time.

With regards to the sed command I provided, I thought sed would be 
"fairly recent" on all systems, but it seems like the version shipped 
with macOS is quite dated (or at least lacks features).
I tried rewriting it with awk but I had the same issue.
Currently I have this perl one which worked fine on both Linux (Ubuntu 
16.04) and macOS 10.13 with a fresh git clone of llvm-mirror.

git grep -l 'DEBUG' | xargs perl -pi -e 's/\bDEBUG\s?\(/LLVM_DEBUG(/g'

Can you test it out and let me know if it works for you? I'd like to 
have one that works on multiple platforms so I can eventually update the 
release notes and email llvm-dev with it (in the "change has been 
completed" email).

Thanks,
Nicola


On 24/05/18 17:43, Daniel Sanders wrote:
> Hi Nicola,
>
> This removal should get an email on llvm-dev and a mention in the 
> release notes since there's a lot of out-of-tree projects that are 
> going to be broken by this change and won't have noticed the 
> llvm-commits post. In my case I can't find the earlier commits that 
> added LLVM_DEBUG in my mailbox so I'm not sure what happened there, 
> maybe it was filtered out for being too large. Some projects will 
> notice when they're pulling the trunk into their downstream repo's 
> (like we did :-) ) but there will also be another wave when we do the 
> next release as there's also out of tree targets that don't closely 
> track trunk.
>
> It's also worth mentioning the merge pain that the very quick removal 
> is going to cause. I'm reluctant to mention this since I believe it 
> isn't LLVM's responsibility to account for it (it's ours) but this 
> case is a bit special in that it's likely to hit an unusually large 
> number of merges over the next few years. As bugs are fixed in 
> existing releases, it's common to merge those fixes into the current 
> code-base so that the bugs aren't introduced again. Effectively that 
> means that some branches are using DEBUG() and others are using 
> LLVM_DEBUG() and the practical result of this is that any merges 
> containing or near lines with the DEBUG() macro are going result in 
> merge conflicts. Probably the least painful way to resolve it is to 
> re-introduce DEBUG() locally but I don't like that solution because 
> we'll never make the switch. Unfortunately making the switch on all 
> branches probably isn't an option either. I'm going to have a think on 
> how best to deal with this.
>
> By the way, the sed command from your earlier commit doesn't work on 
> macOS because -i has a mandatory argument and \b isn't supported. Even 
> with those issues fixed I end up with:
> sed: RE error: illegal byte sequence
>
> This seems to work though:
> for f in $(grep -rlw --exclude-dir=.git 'DEBUG' .);
> do
>   echo $f
>   sed -i '.bak' -e 's/[[:<:]]DEBUG[[:>:]][ \t]*(/LLVM_DEBUG(/g' $f;
> done
>
>> On 23 May 2018, at 08:09, Nicola Zaghen via llvm-commits 
>> <llvm-commits at lists.llvm.org <mailto:llvm-commits at lists.llvm.org>> wrote:
>>
>> Author: nzaghen
>> Date: Wed May 23 08:09:29 2018
>> New Revision: 333091
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=333091&view=rev
>> Log:
>> Remove DEBUG macro.
>>
>> Now that the LLVM_DEBUG() macro landed on the various sub-projects
>> the DEBUG macro can be removed.
>> Also change the new uses of DEBUG to LLVM_DEBUG.
>>
>> Differential Revision: https://reviews.llvm.org/D46952
>>
>>
>> Modified:
>>    llvm/trunk/include/llvm/DebugInfo/PDB/DIA/DIASupport.h
>>    llvm/trunk/include/llvm/Support/Debug.h
>>    llvm/trunk/lib/Target/X86/X86DomainReassignment.cpp
>>    llvm/trunk/lib/Transforms/IPO/MergeFunctions.cpp
>>    llvm/trunk/lib/Transforms/Vectorize/VPlanHCFGBuilder.cpp
>>    llvm/trunk/lib/Transforms/Vectorize/VPlanVerifier.cpp
>>    llvm/trunk/utils/TableGen/GlobalISelEmitter.cpp
>>
>> Modified: llvm/trunk/include/llvm/DebugInfo/PDB/DIA/DIASupport.h
>> URL: 
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/DebugInfo/PDB/DIA/DIASupport.h?rev=333091&r1=333090&r2=333091&view=diff
>> ==============================================================================
>> --- llvm/trunk/include/llvm/DebugInfo/PDB/DIA/DIASupport.h (original)
>> +++ llvm/trunk/include/llvm/DebugInfo/PDB/DIA/DIASupport.h Wed May 23 
>> 08:09:29 2018
>> @@ -22,14 +22,6 @@
>> #define NOMINMAX
>> #endif
>>
>> -// llvm/Support/Debug.h unconditionally #defines DEBUG as a macro.
>> -// DIA headers #define it if it is not already defined, so we have
>> -// an order of includes problem.  The real fix is to make LLVM use
>> -// something less generic than DEBUG, such as LLVM_DEBUG(), but it's
>> -// fairly prevalent.  So for now, we save the definition state and
>> -// restore it.
>> -#pragma push_macro("DEBUG")
>> -
>> // atlbase.h has to come before windows.h
>> #include <atlbase.h>
>> #include <windows.h>
>> @@ -39,6 +31,4 @@
>> #include <dia2.h>
>> #include <diacreate.h>
>>
>> -#pragma pop_macro("DEBUG")
>> -
>> #endif // LLVM_DEBUGINFO_PDB_DIA_DIASUPPORT_H
>>
>> Modified: llvm/trunk/include/llvm/Support/Debug.h
>> URL: 
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Support/Debug.h?rev=333091&r1=333090&r2=333091&view=diff
>> ==============================================================================
>> --- llvm/trunk/include/llvm/Support/Debug.h (original)
>> +++ llvm/trunk/include/llvm/Support/Debug.h Wed May 23 08:09:29 2018
>> @@ -118,8 +118,6 @@ raw_ostream &dbgs();
>> //
>> #define LLVM_DEBUG(X) DEBUG_WITH_TYPE(DEBUG_TYPE, X)
>>
>> -#define DEBUG(X) LLVM_DEBUG(X)
>> -
>> } // end namespace llvm
>>
>> #endif // LLVM_SUPPORT_DEBUG_H
>>
>> Modified: llvm/trunk/lib/Target/X86/X86DomainReassignment.cpp
>> URL: 
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/X86DomainReassignment.cpp?rev=333091&r1=333090&r2=333091&view=diff
>> ==============================================================================
>> --- llvm/trunk/lib/Target/X86/X86DomainReassignment.cpp (original)
>> +++ llvm/trunk/lib/Target/X86/X86DomainReassignment.cpp Wed May 23 
>> 08:09:29 2018
>> @@ -753,7 +753,7 @@ bool X86DomainReassignment::runOnMachine
>>   }
>>
>>   for (Closure &C : Closures) {
>> -    DEBUG(C.dump(MRI));
>> +    LLVM_DEBUG(C.dump(MRI));
>>     if (isReassignmentProfitable(C, MaskDomain)) {
>>       reassign(C, MaskDomain);
>>       ++NumClosuresConverted;
>>
>> Modified: llvm/trunk/lib/Transforms/IPO/MergeFunctions.cpp
>> URL: 
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/IPO/MergeFunctions.cpp?rev=333091&r1=333090&r2=333091&view=diff
>> ==============================================================================
>> --- llvm/trunk/lib/Transforms/IPO/MergeFunctions.cpp (original)
>> +++ llvm/trunk/lib/Transforms/IPO/MergeFunctions.cpp Wed May 23 
>> 08:09:29 2018
>> @@ -645,8 +645,8 @@ void MergeFunctions::filterInstsUnrelate
>> static bool isThunkProfitable(Function * F) {
>>   if (F->size() == 1) {
>>     if (F->front().size() <= 2) {
>> -      DEBUG(dbgs() << "isThunkProfitable: " << F->getName()
>> -                    << " is too small to bother creating a thunk 
>> for\n");
>> +      LLVM_DEBUG(dbgs() << "isThunkProfitable: " << F->getName()
>> +                        << " is too small to bother creating a thunk 
>> for\n");
>>       return false;
>>     }
>>   }
>>
>> Modified: llvm/trunk/lib/Transforms/Vectorize/VPlanHCFGBuilder.cpp
>> URL: 
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Vectorize/VPlanHCFGBuilder.cpp?rev=333091&r1=333090&r2=333091&view=diff
>> ==============================================================================
>> --- llvm/trunk/lib/Transforms/Vectorize/VPlanHCFGBuilder.cpp (original)
>> +++ llvm/trunk/lib/Transforms/Vectorize/VPlanHCFGBuilder.cpp Wed May 
>> 23 08:09:29 2018
>> @@ -116,7 +116,7 @@ VPBasicBlock *PlainCFGBuilder::getOrCrea
>>     return BlockIt->second;
>>
>>   // Create new VPBB.
>> -  DEBUG(dbgs() << "Creating VPBasicBlock for " << BB->getName() << 
>> "\n");
>> +  LLVM_DEBUG(dbgs() << "Creating VPBasicBlock for " << BB->getName() 
>> << "\n");
>>   VPBasicBlock *VPBB = new VPBasicBlock(BB->getName());
>>   BB2VPBB[BB] = VPBB;
>>   VPBB->setParent(TopRegion);
>> @@ -314,7 +314,7 @@ void VPlanHCFGBuilder::buildHierarchical
>>   PlainCFGBuilder PCFGBuilder(TheLoop, LI, Plan);
>>   VPRegionBlock *TopRegion = PCFGBuilder.buildPlainCFG();
>>   Plan.setEntry(TopRegion);
>> -  DEBUG(Plan.setName("HCFGBuilder: Plain CFG\n"); dbgs() << Plan);
>> +  LLVM_DEBUG(Plan.setName("HCFGBuilder: Plain CFG\n"); dbgs() << Plan);
>>
>>   Verifier.verifyHierarchicalCFG(TopRegion);
>> }
>>
>> Modified: llvm/trunk/lib/Transforms/Vectorize/VPlanVerifier.cpp
>> URL: 
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Vectorize/VPlanVerifier.cpp?rev=333091&r1=333090&r2=333091&view=diff
>> ==============================================================================
>> --- llvm/trunk/lib/Transforms/Vectorize/VPlanVerifier.cpp (original)
>> +++ llvm/trunk/lib/Transforms/Vectorize/VPlanVerifier.cpp Wed May 23 
>> 08:09:29 2018
>> @@ -120,7 +120,7 @@ void VPlanVerifier::verifyHierarchicalCF
>>   if (!EnableHCFGVerifier)
>>     return;
>>
>> -  DEBUG(dbgs() << "Verifying VPlan H-CFG.\n");
>> +  LLVM_DEBUG(dbgs() << "Verifying VPlan H-CFG.\n");
>>   assert(!TopRegion->getParent() && "VPlan Top Region should have no 
>> parent.");
>>   verifyRegionRec(TopRegion);
>> }
>>
>> Modified: llvm/trunk/utils/TableGen/GlobalISelEmitter.cpp
>> URL: 
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/utils/TableGen/GlobalISelEmitter.cpp?rev=333091&r1=333090&r2=333091&view=diff
>> ==============================================================================
>> --- llvm/trunk/utils/TableGen/GlobalISelEmitter.cpp (original)
>> +++ llvm/trunk/utils/TableGen/GlobalISelEmitter.cpp Wed May 23 
>> 08:09:29 2018
>> @@ -4063,7 +4063,7 @@ std::vector<Matcher *> GlobalISelEmitter
>>   }
>>   ProcessCurrentGroup();
>>
>> -  DEBUG(dbgs() << "NumGroups: " << NumGroups << "\n");
>> +  LLVM_DEBUG(dbgs() << "NumGroups: " << NumGroups << "\n");
>>   assert(CurrentGroup->empty() && "The last group wasn't properly 
>> processed");
>>   return OptRules;
>> }
>>
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at lists.llvm.org <mailto:llvm-commits at lists.llvm.org>
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180525/fd708a6d/attachment-0001.html>


More information about the llvm-commits mailing list