[PATCH] D114104: [FIR] Convert fir.allocmem and fir.freemem operations to calls to malloc and free, respectively

Alexis Perry-Holby via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Dec 6 14:59:27 PST 2021


AlexisPerry added a comment.

Thanks @awarzynski !  Yes, the namespace changes (and use of "static") were requested in a previous comment on this review :

> The coding standard indicates that anonymous namespace should be made as small as possible and only include class declarations.
> Functions should be made static and outside the namespace instead.
>
> https://llvm.org/docs/CodingStandards.html#anonymous-namespaces

Since this is clearly a case of making something match the LLVM coding standards (and the original code on fir-dev that this patch was taken from matched this particular part of the standards to begin with), I see no need to split these simple changes out into a separate patch.



================
Comment at: flang/lib/Optimizer/CodeGen/CodeGen.cpp:223
   mlir::LogicalResult
-  matchAndRewrite(fir::AbsentOp absent, OpAdaptor,
+  matchAndRewrite(fir::AbsentOp absent, OpAdaptor adaptor,
                   mlir::ConversionPatternRewriter &rewriter) const override {
----------------
awarzynski wrote:
> AlexisPerry wrote:
> > clementval wrote:
> > > Is this change needed?
> > I couldn't build the code without this change, so I would say yes?
> @AlexisPerry This is an unrelated change and IMO it shouldn't be included here.  Perhaps earlier you were getting build errors due to some other issues? Could you try again? If you are getting build errors, could you share the log with us?
I had been getting build errors on my local machine specifically calling out this line and complaining that there was no variable name provided.  However, now these build errors have magically disappeared, so I am updating the diff accordingly.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D114104/new/

https://reviews.llvm.org/D114104



More information about the llvm-commits mailing list