628 lines · plain
1=======================================================2How to Update Debug Info: A Guide for LLVM Pass Authors3=======================================================4 5.. contents::6 :local:7 8Introduction9============10 11Certain kinds of code transformations can inadvertently result in a loss of12debug info, or worse, make debug info misrepresent the state of a program. Debug13info availability is also essential for SamplePGO.14 15This document specifies how to correctly update debug info in various kinds of16code transformations, and offers suggestions for how to create targeted debug17info tests for arbitrary transformations.18 19For more on the philosophy behind LLVM debugging information, see20:doc:`SourceLevelDebugging`.21 22Rules for updating debug locations23==================================24 25.. _WhenToPreserveLocation:26 27When to preserve an instruction location28----------------------------------------29 30A transformation should preserve the debug location of an instruction if the31instruction either remains in its basic block, or if its basic block is folded32into a predecessor that branches unconditionally. The APIs to use are33``IRBuilder``, or ``Instruction::setDebugLoc``.34 35The purpose of this rule is to ensure that common block-local optimizations36preserve the ability to set breakpoints on source locations corresponding to37the instructions they touch. Debugging, crash logs, and SamplePGO accuracy38would be severely impacted if that ability were lost.39 40Examples of transformations that should follow this rule include:41 42* Instruction scheduling. Block-local instruction reordering should not drop43 source locations, even though this may lead to jumpy single-stepping44 behavior.45 46* Simple jump threading. For example, if block ``B1`` unconditionally jumps to47 ``B2``, *and* is its unique predecessor, instructions from ``B2`` can be48 hoisted into ``B1``. Source locations from ``B2`` should be preserved.49 50* Peephole optimizations that replace or expand an instruction, like ``(add X51 X) => (shl X 1)``. The location of the ``shl`` instruction should be the same52 as the location of the ``add`` instruction.53 54* Tail duplication. For example, if blocks ``B1`` and ``B2`` both55 unconditionally branch to ``B3`` and ``B3`` can be folded into its56 predecessors, source locations from ``B3`` should be preserved.57 58Examples of transformations for which this rule *does not* apply include:59 60* LICM. E.g., if an instruction is moved from the loop body to the preheader,61 the rule for :ref:`dropping locations<WhenToDropLocation>` applies.62 63In addition to the rule above, a transformation should also preserve the debug64location of an instruction that is moved between basic blocks, if the65destination block already contains an instruction with an identical debug66location.67 68Examples of transformations that should follow this rule include:69 70* Moving instructions between basic blocks. For example, if instruction ``I1``71 in ``BB1`` is moved before ``I2`` in ``BB2``, the source location of ``I1``72 can be preserved if it has the same source location as ``I2``.73 74.. _WhenToMergeLocation:75 76When to merge instruction locations77-----------------------------------78 79A transformation should merge instruction locations if it replaces multiple80instructions with one or more new instructions, *and* the new instruction(s)81produce the output of more than one of the original instructions. The API to use82is ``Instruction::applyMergedLocation``. For each new instruction I, its new83location should be a merge of the locations of all instructions whose output is84produced by I. Typically, this includes any instruction being RAUWed by a new85instruction, and excludes any instruction that only produces an intermediate86value used by the RAUWed instruction.87 88The purpose of this rule is to ensure that a) the single merged instruction89has a location with an accurate scope attached, and b) to prevent misleading90single-stepping (or breakpoint) behavior. Often, merged instructions are memory91accesses which can trap: having an accurate scope attached greatly assists in92crash triage by identifying the (possibly inlined) function where the bad93memory access occurred.94 95To maintain distinct source locations for SamplePGO, it is often beneficial to96retain an arbitrary but deterministic location instead of discarding line and97column information as part of merging. In particular, loss of location98information for calls inhibits optimizations such as indirect call promotion.99This behavior can be optionally enabled until support for accurately100representing merged instructions in the line table is implemented.101 102Examples of transformations that should follow this rule include:103 104* Hoisting identical instructions from all successors of a conditional branch105 or sinking those from all paths to a postdominating block. For example,106 merging identical loads/stores which occur on both sides of a CFG diamond107 (see the ``MergedLoadStoreMotion`` pass). For each group of identical108 instructions being hoisted/sunk, the merge of all their locations should be109 applied to the merged instruction.110 111* Merging identical loop-invariant stores (see the LICM utility112 ``llvm::promoteLoopAccessesToScalars``).113 114* Scalar instructions being combined into a vector instruction, like115 ``(add A1, B1), (add A2, B2) => (add (A1, A2), (B1, B2))``. As the new vector116 ``add`` computes the result of both original ``add`` instructions117 simultaneously, it should use a merge of the two locations. Similarly, if118 prior optimizations have already produced vectors ``(A1, A2)`` and119 ``(B2, B1)``, then we might create a ``(shufflevector (1, 0), (B2, B1))``120 instruction to produce ``(B1, B2)`` for the vector ``add``; in this case we've121 created two instructions to replace the original ``adds``, so both new122 instructions should use the merged location.123 124Examples of transformations for which this rule *does not* apply include:125 126* Block-local peepholes which delete redundant instructions, like127 ``(sext (zext i8 %x to i16) to i32) => (zext i8 %x to i32)``. The inner128 ``zext`` is modified but remains in its block, so the rule for129 :ref:`preserving locations<WhenToPreserveLocation>` should apply.130 131* Peephole optimizations which combine multiple instructions together, like132 ``(add (mul A B) C) => llvm.fma.f32(A, B, C)``. Note that the result of the133 ``mul`` no longer appears in the program, while the result of the ``add`` is134 now produced by the ``fma``, so the ``add``'s location should be used.135 136* Converting an if-then-else CFG diamond into a ``select``. Preserving the137 debug locations of speculated instructions can make it seem like a condition138 is true when it's not (or vice versa), which leads to a confusing139 single-stepping experience. The rule for140 :ref:`dropping locations<WhenToDropLocation>` should apply here.141 142* Hoisting/sinking that would make a location reachable when it previously143 wasn't. Consider hoisting two identical instructions with the same location144 from first two cases of a switch that has three cases. Merging their145 locations would make the location from the first two cases reachable when the146 third case is taken. The rule for147 :ref:`dropping locations<WhenToDropLocation>` applies.148 149.. _WhenToDropLocation:150 151When to drop an instruction location152------------------------------------153 154A transformation should drop debug locations if the rules for155:ref:`preserving<WhenToPreserveLocation>` and156:ref:`merging<WhenToMergeLocation>` debug locations do not apply. The API to157use is ``Instruction::dropLocation()``.158 159The purpose of this rule is to prevent erratic or misleading single-stepping160behavior in situations in which an instruction has no clear, unambiguous161relationship to a source location.162 163To handle an instruction without a location, the DWARF generator164defaults to allowing the last-set location after a label to cascade forward, or165to setting a line 0 location with viable scope information if no previous166location is available.167 168See the discussion in the section about169:ref:`merging locations<WhenToMergeLocation>` for examples of when the rule for170dropping locations applies.171 172When to remap a debug location173------------------------------174 175When code paths are duplicated, during passes such as loop unrolling or jump176threading, `DILocation` attachments need to be remapped using `mapAtomInstance`177and `RemapSourceAtom`. This is to support the Key Instructions debug info feature.178See :doc:`KeyInstructionsDebugInfo` for information.179 180.. _NewInstLocations:181 182Setting locations for new instructions183--------------------------------------184 185Whenever a new instruction is created and there is no suitable location for that186instruction, that instruction should be annotated accordingly. There are a set187of special ``DebugLoc`` values that can be set on an instruction to annotate the188reason that it does not have a valid location. These are as follows:189 190* ``DebugLoc::getCompilerGenerated()``: This indicates that the instruction is a191 compiler-generated instruction, i.e. it is not associated with any user source192 code.193 194* ``DebugLoc::getDropped()``: This indicates that the instruction has195 intentionally had its source location removed, according to the rules for196 :ref:`dropping locations<WhenToDropLocation>`; this is set automatically by197 ``Instruction::dropLocation()``.198 199* ``DebugLoc::getUnknown()``: This indicates that the instruction does not have200 a known or currently knowable source location, e.g. that it is infeasible to201 determine the correct source location, or that the source location is202 ambiguous in a way that LLVM cannot currently represent.203 204* ``DebugLoc::getTemporary()``: This is used for instructions that we don't205 expect to be emitted (e.g. ``UnreachableInst``), and so should not need a206 valid location; if we ever try to emit a temporary location into an object/asm207 file, this indicates that something has gone wrong.208 209Where applicable, these should be used instead of leaving an instruction without210an assigned location or explicitly setting the location as ``DebugLoc()``.211Ordinarily these special locations are identical to an absent location, but LLVM212built with coverage-tracking213(``-DLLVM_ENABLE_DEBUGLOC_COVERAGE_TRACKING="COVERAGE"``) will keep track of214these special locations in order to detect unintentionally-missing locations;215for this reason, the most important rule is to *not* apply any of these if it216isn't clear which, if any, is appropriate - an absent location can be detected217and fixed, while an incorrectly annotated instruction is much harder to detect.218On the other hand, if any of these clearly apply, then they should be used to219prevent false positives from being flagged up.220 221Rules for updating debug values222===============================223 224Deleting an IR-level Instruction225--------------------------------226 227When an ``Instruction`` is deleted, its debug uses change to ``undef``. This is228a loss of debug info: the value of one or more source variables becomes229unavailable, starting with the ``#dbg_value(undef, ...)``. When there is no230way to reconstitute the value of the lost instruction, this is the best231possible outcome. However, it's often possible to do better:232 233* If the dying instruction can be RAUW'd, do so. The234 ``Value::replaceAllUsesWith`` API transparently updates debug uses of the235 dying instruction to point to the replacement value.236 237* If the dying instruction cannot be RAUW'd, call ``llvm::salvageDebugInfo`` on238 it. This makes a best-effort attempt to rewrite debug uses of the dying239 instruction by describing its effect as a ``DIExpression``.240 241* If one of the **operands** of a dying instruction would become trivially242 dead, use ``llvm::replaceAllDbgUsesWith`` to rewrite the debug uses of that243 operand. Consider the following example function:244 245.. code-block:: llvm246 247 define i16 @foo(i16 %a) {248 %b = sext i16 %a to i32249 %c = and i32 %b, 15250 #dbg_value(i32 %c, ...)251 %d = trunc i32 %c to i16252 ret i16 %d253 }254 255Now, here's what happens after the unnecessary truncation instruction ``%d`` is256replaced with a simplified instruction:257 258.. code-block:: llvm259 260 define i16 @foo(i16 %a) {261 #dbg_value(i32 undef, ...)262 %simplified = and i16 %a, 15263 ret i16 %simplified264 }265 266Note that after deleting ``%d``, all uses of its operand ``%c`` become267trivially dead. The debug use which used to point to ``%c`` is now ``undef``,268and debug info is needlessly lost.269 270To solve this problem, do:271 272.. code-block:: cpp273 274 llvm::replaceAllDbgUsesWith(%c, theSimplifiedAndInstruction, ...)275 276This results in better debug info because the debug use of ``%c`` is preserved:277 278.. code-block:: llvm279 280 define i16 @foo(i16 %a) {281 %simplified = and i16 %a, 15282 #dbg_value(i16 %simplified, ...)283 ret i16 %simplified284 }285 286You may have noticed that ``%simplified`` is narrower than ``%c``: this is not287a problem, because ``llvm::replaceAllDbgUsesWith`` takes care of inserting the288necessary conversion operations into the DIExpressions of updated debug uses.289 290Deleting a MIR-level MachineInstr291---------------------------------292 293TODO294 295Rules for updating ``DIAssignID`` Attachments296=============================================297 298``DIAssignID`` metadata attachments are used by Assignment Tracking, which is299currently an experimental debug mode.300 301See :doc:`AssignmentTracking` for how to update them and for more info on302Assignment Tracking.303 304How to automatically convert tests into debug info tests305========================================================306 307.. _IRDebugify:308 309Mutation testing for IR-level transformations310---------------------------------------------311 312An IR test case for a transformation can, in many cases, be automatically313mutated to test debug info handling within that transformation. This is a314simple way to test for proper debug info handling.315 316The ``debugify`` utility pass317^^^^^^^^^^^^^^^^^^^^^^^^^^^^^318 319The ``debugify`` testing utility is just a pair of passes: ``debugify`` and320``check-debugify``.321 322The first applies synthetic debug information to every instruction of the323module, and the second checks that this DI is still available after an324optimization has occurred, reporting any errors/warnings while doing so.325 326The instructions are assigned sequentially increasing line locations, and are327immediately used by debug value records everywhere possible.328 329For example, here is a module before:330 331.. code-block:: llvm332 333 define void @f(i32* %x) {334 entry:335 %x.addr = alloca i32*, align 8336 store i32* %x, i32** %x.addr, align 8337 %0 = load i32*, i32** %x.addr, align 8338 store i32 10, i32* %0, align 4339 ret void340 }341 342and after running ``opt -debugify``:343 344.. code-block:: llvm345 346 define void @f(i32* %x) !dbg !6 {347 entry:348 %x.addr = alloca i32*, align 8, !dbg !12349 #dbg_value(i32** %x.addr, !9, !DIExpression(), !12)350 store i32* %x, i32** %x.addr, align 8, !dbg !13351 %0 = load i32*, i32** %x.addr, align 8, !dbg !14352 #dbg_value(i32* %0, !11, !DIExpression(), !14)353 store i32 10, i32* %0, align 4, !dbg !15354 ret void, !dbg !16355 }356 357 !llvm.dbg.cu = !{!0}358 !llvm.debugify = !{!3, !4}359 !llvm.module.flags = !{!5}360 361 !0 = distinct !DICompileUnit(language: DW_LANG_C, file: !1, producer: "debugify", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, enums: !2)362 !1 = !DIFile(filename: "debugify-sample.ll", directory: "/")363 !2 = !{}364 !3 = !{i32 5}365 !4 = !{i32 2}366 !5 = !{i32 2, !"Debug Info Version", i32 3}367 !6 = distinct !DISubprogram(name: "f", linkageName: "f", scope: null, file: !1, line: 1, type: !7, isLocal: false, isDefinition: true, scopeLine: 1, isOptimized: true, unit: !0, retainedNodes: !8)368 !7 = !DISubroutineType(types: !2)369 !8 = !{!9, !11}370 !9 = !DILocalVariable(name: "1", scope: !6, file: !1, line: 1, type: !10)371 !10 = !DIBasicType(name: "ty64", size: 64, encoding: DW_ATE_unsigned)372 !11 = !DILocalVariable(name: "2", scope: !6, file: !1, line: 3, type: !10)373 !12 = !DILocation(line: 1, column: 1, scope: !6)374 !13 = !DILocation(line: 2, column: 1, scope: !6)375 !14 = !DILocation(line: 3, column: 1, scope: !6)376 !15 = !DILocation(line: 4, column: 1, scope: !6)377 !16 = !DILocation(line: 5, column: 1, scope: !6)378 379Using ``debugify``380^^^^^^^^^^^^^^^^^^381 382A simple way to use ``debugify`` is as follows:383 384.. code-block:: bash385 386 $ opt -debugify -pass-to-test -check-debugify sample.ll387 388This will inject synthetic DI to ``sample.ll`` run the ``pass-to-test`` and389then check for missing DI. The ``-check-debugify`` step can of course be390omitted in favor of more customizable FileCheck directives.391 392Some other ways to run debugify are available:393 394.. code-block:: bash395 396 # Same as the above example.397 $ opt -enable-debugify -pass-to-test sample.ll398 399 # Suppresses verbose debugify output.400 $ opt -enable-debugify -debugify-quiet -pass-to-test sample.ll401 402 # Prepend -debugify before and append -check-debugify -strip after403 # each pass on the pipeline (similar to -verify-each).404 $ opt -debugify-each -O2 sample.ll405 406In order for ``check-debugify`` to work, the DI must be coming from407``debugify``. Thus, modules with existing DI will be skipped.408 409``debugify`` can be used to test a backend, e.g:410 411.. code-block:: bash412 413 $ opt -debugify < sample.ll | llc -o -414 415There is also a MIR-level debugify pass that can be run before each backend416pass, see:417:ref:`Mutation testing for MIR-level transformations<MIRDebugify>`.418 419``debugify`` in regression tests420^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^421 422The output of the ``debugify`` pass must be stable enough to use in regression423tests. Changes to this pass are not allowed to break existing tests.424 425.. note::426 427 Regression tests must be robust. Avoid hardcoding line/variable numbers in428 check lines. In cases where this can't be avoided (say, if a test wouldn't429 be precise enough), moving the test to its own file is preferred.430 431Using Coverage Tracking to remove false positives432^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^433 434As described :ref:`above<WhenToDropLocation>`, there are valid reasons for435instructions to not have source locations. Therefore, when detecting dropped or436not-generated source locations, it may be preferable to avoid detecting cases437where the missing source location is intentional. For this, you can use the438"coverage tracking" feature in LLVM to prevent these from appearing in the439``debugify`` output. This is enabled in a build of LLVM by setting the CMake440flag ``-DLLVM_ENABLE_DEBUGLOC_COVERAGE_TRACKING=COVERAGE``. When this has been441set, LLVM will enable runtime tracking of442:ref:`DebugLoc annotations<NewInstLocations>`, allowing ``debugify`` to ignore443instructions that have an explicitly recorded reason given for not having a444source location.445 446For triaging source location bugs detected with ``debugify``, you may find it447helpful to instead set the CMake flag to enable "origin tracking",448``-DLLVM_ENABLE_DEBUGLOC_COVERAGE_TRACKING=COVERAGE_AND_ORIGIN``. This flag adds449more detail to ``debugify``'s output, by including one or more stacktraces with450every missing source location, capturing the point at which the empty source451location was created, and every point at which it was copied to an instruction,452making it trivial in most cases to find the origin of the underlying bug. If453using origin tracking, it is recommended to also build LLVM with debug info454enabled, so that the stacktrace can be accurately symbolized.455 456.. note::457 458 The coverage tracking feature has been designed primarily for use with the459 :ref:`original debug info preservation<OriginalDI>` mode of ``debugify``, and460 so may not be reliable in other settings. When using this mode, the461 stacktraces produced by the ``COVERAGE_AND_ORIGIN`` setting will be printed462 in an easy-to-read format as part of the reports generated by the463 ``llvm-original-di-preservation.py`` script.464 465.. _OriginalDI:466 467Test original debug info preservation in optimizations468------------------------------------------------------469 470In addition to automatically generating debug info, the checks provided by471the ``debugify`` utility pass can also be used to test the preservation of472pre-existing debug info metadata. It could be run as follows:473 474.. code-block:: bash475 476 # Run the pass by checking original Debug Info preservation.477 $ opt -verify-debuginfo-preserve -pass-to-test sample.ll478 479 # Check the preservation of original Debug Info after each pass.480 $ opt -verify-each-debuginfo-preserve -O2 sample.ll481 482Limit number of observed functions to speed up the analysis:483 484.. code-block:: bash485 486 # Test up to 100 functions (per compile unit) per pass.487 $ opt -verify-each-debuginfo-preserve -O2 -debugify-func-limit=100 sample.ll488 489Please do note that running ``-verify-each-debuginfo-preserve`` on big projects490could be heavily time consuming. Therefore, we suggest using491``-debugify-func-limit`` with a suitable limit number to prevent extremely long492builds.493 494Furthermore, there is a way to export the issues that have been found into495a JSON file as follows:496 497.. code-block:: bash498 499 $ opt -verify-debuginfo-preserve -verify-di-preserve-export=sample.json -pass-to-test sample.ll500 501and then use the ``llvm/utils/llvm-original-di-preservation.py`` script502to generate an HTML page with the issues reported in a more human-readable form503as follows:504 505.. code-block:: bash506 507 $ llvm-original-di-preservation.py sample.json --report-file sample.html508 509Testing of original debug info preservation can be invoked from front-end level510as follows:511 512.. code-block:: bash513 514 # Test each pass.515 $ clang -Xclang -fverify-debuginfo-preserve -g -O2 sample.c516 517 # Test each pass and export the issues report into the JSON file.518 $ clang -Xclang -fverify-debuginfo-preserve -Xclang -fverify-debuginfo-preserve-export=sample.json -g -O2 sample.c519 520Please do note that there are some known false positives, for source locations521and debug record checking, so that will be addressed as a future work.522 523.. _MIRDebugify:524 525Mutation testing for MIR-level transformations526----------------------------------------------527 528A variant of the ``debugify`` utility described in529:ref:`Mutation testing for IR-level transformations<IRDebugify>` can be used530for MIR-level transformations as well: much like the IR-level pass,531``mir-debugify`` inserts sequentially increasing line locations to each532``MachineInstr`` in a ``Module``. And the MIR-level ``mir-check-debugify`` is533similar to IR-level ``check-debugify`` pass.534 535For example, here is a snippet before:536 537.. code-block:: llvm538 539 name: test540 body: |541 bb.1 (%ir-block.0):542 %0:_(s32) = IMPLICIT_DEF543 %1:_(s32) = IMPLICIT_DEF544 %2:_(s32) = G_CONSTANT i32 2545 %3:_(s32) = G_ADD %0, %2546 %4:_(s32) = G_SUB %3, %1547 548and after running ``llc -run-pass=mir-debugify``:549 550.. code-block:: llvm551 552 name: test553 body: |554 bb.0 (%ir-block.0):555 %0:_(s32) = IMPLICIT_DEF debug-location !12556 DBG_VALUE %0(s32), $noreg, !9, !DIExpression(), debug-location !12557 %1:_(s32) = IMPLICIT_DEF debug-location !13558 DBG_VALUE %1(s32), $noreg, !11, !DIExpression(), debug-location !13559 %2:_(s32) = G_CONSTANT i32 2, debug-location !14560 DBG_VALUE %2(s32), $noreg, !9, !DIExpression(), debug-location !14561 %3:_(s32) = G_ADD %0, %2, debug-location !DILocation(line: 4, column: 1, scope: !6)562 DBG_VALUE %3(s32), $noreg, !9, !DIExpression(), debug-location !DILocation(line: 4, column: 1, scope: !6)563 %4:_(s32) = G_SUB %3, %1, debug-location !DILocation(line: 5, column: 1, scope: !6)564 DBG_VALUE %4(s32), $noreg, !9, !DIExpression(), debug-location !DILocation(line: 5, column: 1, scope: !6)565 566By default, ``mir-debugify`` inserts ``DBG_VALUE`` instructions **everywhere**567it is legal to do so. In particular, every (non-PHI) machine instruction that568defines a register must be followed by a ``DBG_VALUE`` use of that def. If569an instruction does not define a register, but can be followed by a debug inst,570MIRDebugify inserts a ``DBG_VALUE`` that references a constant. Insertion of571``DBG_VALUE``'s can be disabled by setting ``-debugify-level=locations``.572 573To run MIRDebugify once, simply insert ``mir-debugify`` into your ``llc``574invocation, like:575 576.. code-block:: bash577 578 # Before some other pass.579 $ llc -run-pass=mir-debugify,other-pass ...580 581 # After some other pass.582 $ llc -run-pass=other-pass,mir-debugify ...583 584To run MIRDebugify before each pass in a pipeline, use585``-debugify-and-strip-all-safe``. This can be combined with ``-start-before``586and ``-start-after``. For example:587 588.. code-block:: bash589 590 $ llc -debugify-and-strip-all-safe -run-pass=... <other llc args>591 $ llc -debugify-and-strip-all-safe -O1 <other llc args>592 593If you want to check it after each pass in a pipeline, use594``-debugify-check-and-strip-all-safe``. This can also be combined with595``-start-before`` and ``-start-after``. For example:596 597.. code-block:: bash598 599 $ llc -debugify-check-and-strip-all-safe -run-pass=... <other llc args>600 $ llc -debugify-check-and-strip-all-safe -O1 <other llc args>601 602To check all debug info from a test, use ``mir-check-debugify``, like:603 604.. code-block:: bash605 606 $ llc -run-pass=mir-debugify,other-pass,mir-check-debugify607 608To strip out all debug info from a test, use ``mir-strip-debug``, like:609 610.. code-block:: bash611 612 $ llc -run-pass=mir-debugify,other-pass,mir-strip-debug613 614It can be useful to combine ``mir-debugify``, ``mir-check-debugify`` and/or615``mir-strip-debug`` to identify backend transformations which break in616the presence of debug info. For example, to run the AArch64 backend tests617with all normal passes "sandwiched" in between MIRDebugify and618MIRStripDebugify mutation passes, run:619 620.. code-block:: bash621 622 $ llvm-lit test/CodeGen/AArch64 -Dllc="llc -debugify-and-strip-all-safe"623 624Using LostDebugLocObserver625--------------------------626 627TODO628