[PATCH] D24425: DebugInfo: support for DWARFv5 DW_AT_alignment attribute

Adrian Prantl via llvm-commits llvm-commits at lists.llvm.org
Tue Oct 4 13:10:21 PDT 2016


aprantl added inline comments.


> DIBuilder.h:212
> +                                            uint64_t StorageOffsetInBits,
> +                                            DINode::DIFlags Flags, DIType *Ty);
>  

This should be a separate NFC commit.

> DwarfCompileUnit.cpp:779
> +  const auto *DIVar = Var.getVariable();
> +  if (DIVar)
> +    if (uint64_t AlignInBytes = DIVar->getAlignInBytes())

Can this ever be 0?

> Dwarf.cpp:152
> +  case DW_AT_alignment:
> +    return "DW_AT_alignment";
>    case DW_AT_MIPS_loop_begin:            return "DW_AT_MIPS_loop_begin";

For consistency with the rest of the file, I would put this on the same line.

> align_c11.ll:52
> +
> +attributes #0 = { nounwind uwtable "correctly-rounded-divide-sqrt-fp-math"="false" "disable-tail-calls"="false" "less-precise-fpmad"="false" "no-frame-pointer-elim"="true" "no-frame-pointer-elim-non-leaf" "no-infs-fp-math"="false" "no-jump-tables"="false" "no-nans-fp-math"="false" "no-signed-zeros-fp-math"="false" "no-trapping-math"="false" "stack-protector-buffer-size"="8" "target-cpu"="x86-64" "target-features"="+fxsr,+mmx,+sse,+sse2,+x87" "unsafe-fp-math"="false" "use-soft-float"="false" }
> +attributes #1 = { nounwind readnone }

We typically get rid of all unnecessary attributes (= everything in quotes).

> align_cpp11.ll:114
> +
> +attributes #0 = { nounwind uwtable "correctly-rounded-divide-sqrt-fp-math"="false" "disable-tail-calls"="false" "less-precise-fpmad"="false" "no-frame-pointer-elim"="true" "no-frame-pointer-elim-non-leaf" "no-infs-fp-math"="false" "no-jump-tables"="false" "no-nans-fp-math"="false" "no-signed-zeros-fp-math"="false" "no-trapping-math"="false" "stack-protector-buffer-size"="8" "target-cpu"="x86-64" "target-features"="+fxsr,+mmx,+sse,+sse2,+x87" "unsafe-fp-math"="false" "use-soft-float"="false" }
> +attributes #1 = { nounwind readnone }

same here

> Cloning.cpp:256
>      // Create a local variable around the alloca
> -    auto *IntType =
> -        DBuilder.createBasicType("int", 32, dwarf::DW_ATE_signed);
> +    auto *IntType = DBuilder.createBasicType("int", 32, dwarf::DW_ATE_signed);
>      auto *E = DBuilder.createExpression();

This should be a separate NFC commit.

https://reviews.llvm.org/D24425





More information about the llvm-commits mailing list